Skip to content

Commit 16d96af

Browse files
Fix repeated subexpr eval & extend error msgs
This patch fixes the wrong assertion in the regex evaluator that made it impossible to have repeated subexpressions. Assume `(a|b)/(a|b)` regex as an example. This RE have two repeated subexpressions `(a|b)`. The e-graph logic would take into account the fact that these expressions are the same and the resulting tree of the regular expression would have the same node for the `(a|b)` expression. Let's get into the problem. Before this patch the LAGraph solver evaluated lhs, rhs first and then combined their results w.r.t. node type. If there were repeated subexpressions the solver failed due to the assertion that every subexpression had not been evaluated yet. This happened to the example: we evaluated LHS `(a|b)` at first and then we try to calculate RHS `(a|b)`. But the corresponding subtree is already calculated and the assertion is fired. Let's remove the wrong assertion and fix the problem. Apart from these fixes, this patch improves error messages. It has been done in terms of determining what is wrong. There is no reason to avoid these improvements.
1 parent 7fc0a70 commit 16d96af

File tree

1 file changed

+20
-19
lines changed

1 file changed

+20
-19
lines changed

experimental/algorithm/LAGraph_RPQMatrix.c

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,13 @@
118118
GrB_Info info = (s); \
119119
if (info != GrB_SUCCESS) \
120120
{ \
121-
fprintf(stderr, "GraphBLAS error: %d\n", info); \
121+
printf("Message: %s\n", msg); \
122+
fprintf(stderr, "GraphBLAS error: %d (%s, %d)\n", info, __FILE__, __LINE__); \
122123
return info; \
123124
} \
124125
}
125126

127+
char msg[LAGRAPH_MSG_LEN] ;
126128

127129
#include <stdbool.h>
128130
#include <stdio.h>
@@ -179,11 +181,7 @@ GrB_Info LAGraph_DestroyRpqMatrixPlan(RPQMatrixPlan *plan)
179181
{
180182
return GrB_SUCCESS ;
181183
}
182-
if (plan->mat != NULL)
183-
{
184-
OK(GrB_Matrix_free(&(plan->mat))) ;
185-
}
186-
if (plan->res_mat != NULL)
184+
if (plan->res_mat != NULL && plan->mat != plan->res_mat)
187185
{
188186
OK(GrB_Matrix_free(&(plan->res_mat))) ;
189187
}
@@ -201,8 +199,8 @@ GrB_Info LAGraph_RPQMatrix_solver(RPQMatrixPlan *plan, char *msg) ;
201199
static GrB_Info LAGraph_RPQMatrixLor(RPQMatrixPlan *plan, char *msg)
202200
{
203201
LG_ASSERT(plan != NULL, GrB_NULL_POINTER) ;
204-
LG_ASSERT(plan->op == RPQ_MATRIX_OP_LOR, GrB_INVALID_VALUE) ;
205-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
202+
LG_ASSERT_MSG(plan->op == RPQ_MATRIX_OP_LOR, GrB_INVALID_VALUE, "operator is not lor") ;
203+
LG_ASSERT_MSG(plan->res_mat == NULL, GrB_INVALID_VALUE, "resulting matrix is already set as lor result") ;
206204

207205
RPQMatrixPlan *lhs = plan->lhs ;
208206
RPQMatrixPlan *rhs = plan->rhs ;
@@ -234,8 +232,8 @@ static GrB_Info LAGraph_RPQMatrixConcat(RPQMatrixPlan *plan, char *msg)
234232
{
235233

236234
LG_ASSERT(plan != NULL, GrB_NULL_POINTER) ;
237-
LG_ASSERT(plan->op == RPQ_MATRIX_OP_CONCAT, GrB_INVALID_VALUE) ;
238-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
235+
LG_ASSERT_MSG(plan->op == RPQ_MATRIX_OP_CONCAT, GrB_INVALID_VALUE, "operator is not concat") ;
236+
LG_ASSERT_MSG(plan->res_mat == NULL, GrB_INVALID_VALUE, "resulting matrix is already set as concat result") ;
239237

240238
RPQMatrixPlan *lhs = plan->lhs ;
241239
RPQMatrixPlan *rhs = plan->rhs ;
@@ -263,14 +261,14 @@ static GrB_Info LAGraph_RPQMatrixConcat(RPQMatrixPlan *plan, char *msg)
263261
static GrB_Info LAGraph_RPQMatrixKleene(RPQMatrixPlan *plan, char *msg)
264262
{
265263
LG_ASSERT(plan != NULL, GrB_NULL_POINTER) ;
266-
LG_ASSERT(plan->op == RPQ_MATRIX_OP_KLEENE, GrB_INVALID_VALUE) ;
267-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
264+
LG_ASSERT_MSG(plan->op == RPQ_MATRIX_OP_KLEENE, GrB_INVALID_VALUE, "operator is not kleene") ;
265+
LG_ASSERT_MSG(plan->res_mat == NULL, GrB_INVALID_VALUE, "resulting matrix is already set for kleene") ;
268266

269267
RPQMatrixPlan *lhs = plan->lhs ;
270268
RPQMatrixPlan *rhs = plan->rhs ;
271269

272270
// Kleene star should have one child. Always right.
273-
LG_ASSERT(lhs == NULL, GrB_INVALID_VALUE) ;
271+
LG_ASSERT_MSG(lhs == NULL, GrB_INVALID_VALUE, "lhs is expected to be NULL for kleene") ;
274272
LG_ASSERT(rhs != NULL, GrB_NULL_POINTER) ;
275273

276274
OK(LAGraph_RPQMatrix_solver(rhs, msg)) ;
@@ -351,8 +349,8 @@ static GrB_Info LAGraph_RPQMatrixKleene(RPQMatrixPlan *plan, char *msg)
351349
static GrB_Info LAGraph_RPQMatrixKleene_L(RPQMatrixPlan *plan, char *msg)
352350
{
353351
LG_ASSERT(plan != NULL, GrB_NULL_POINTER) ;
354-
LG_ASSERT(plan->op == RPQ_MATRIX_OP_KLEENE_L, GrB_INVALID_VALUE) ;
355-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
352+
LG_ASSERT_MSG(plan->op == RPQ_MATRIX_OP_KLEENE_L, GrB_INVALID_VALUE, "different operator is not expected left-kleene") ;
353+
LG_ASSERT_MSG(plan->res_mat == NULL, GrB_INVALID_VALUE, "resulting matrix is already set for left-kleene") ;
356354

357355
RPQMatrixPlan *lhs = plan->lhs ; // A
358356
RPQMatrixPlan *rhs = plan->rhs ; // B
@@ -426,8 +424,8 @@ static GrB_Info LAGraph_RPQMatrixKleene_L(RPQMatrixPlan *plan, char *msg)
426424
static GrB_Info LAGraph_RPQMatrixKleene_R(RPQMatrixPlan *plan, char *msg)
427425
{
428426
LG_ASSERT(plan != NULL, GrB_NULL_POINTER) ;
429-
LG_ASSERT(plan->op == RPQ_MATRIX_OP_KLEENE_R, GrB_INVALID_VALUE) ;
430-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
427+
LG_ASSERT_MSG(plan->op == RPQ_MATRIX_OP_KLEENE_R, GrB_INVALID_VALUE, "different operator is not expected right-kleene") ;
428+
LG_ASSERT_MSG(plan->res_mat == NULL, GrB_INVALID_VALUE, "resulting matrix is already set for right-kleene") ;
431429

432430
RPQMatrixPlan *lhs = plan->lhs ; // A
433431
RPQMatrixPlan *rhs = plan->rhs ; // B
@@ -473,7 +471,10 @@ static GrB_Info LAGraph_RPQMatrixKleene_R(RPQMatrixPlan *plan, char *msg)
473471

474472
GrB_Info LAGraph_RPQMatrix_solver(RPQMatrixPlan *plan, char *msg)
475473
{
476-
LG_ASSERT(plan->res_mat == NULL, GrB_INVALID_VALUE) ;
474+
if (plan->res_mat != NULL)
475+
{
476+
return (GrB_SUCCESS) ;
477+
}
477478

478479
switch (plan->op)
479480
{
@@ -500,7 +501,7 @@ GrB_Info LAGraph_RPQMatrix_solver(RPQMatrixPlan *plan, char *msg)
500501

501502
GrB_Info LAGraph_RPQMatrix_initialize(void)
502503
{
503-
sr = GxB_ANY_PAIR_BOOL ;
504+
sr = LAGraph_any_one_bool ;
504505
op = GxB_ANY_BOOL_MONOID ;
505506
return GrB_SUCCESS;
506507
}

0 commit comments

Comments
 (0)