Summary
Severity: HIGH
Category: Input mutation / cached-state corruption
Location: experimental/algorithm/LAGr_PartitionQuality.c lines 88–91
Trigger
Call LAGr_PartitionQuality on any graph that has self-edges.
Root Cause
GrB_Matrix A = G->A creates a direct alias to the graph's adjacency matrix. Line 91 then calls:
GRB_TRY(GrB_select(A, NULL, NULL, GrB_OFFDIAG, A, 0, NULL));
GrB_select with output == input performs an in-place operation, permanently deleting self-edges from G->A as a side effect of computing what is supposed to be a read-only quality metric. The cached fields (G->nself_edges, G->out_degree, etc.) are not invalidated afterward.
Compare with LAGr_Modularity, which correctly creates a local copy of G->A before removing self-edges.
Proof / Trace
- Caller holds graph
G with self-loop (0,0) and cached G->nself_edges = 1.
LAGr_PartitionQuality is called; line 91 runs GrB_select(A, NULL, NULL, GrB_OFFDIAG, A, ...).
- Self-edge
(0,0) is permanently removed from G->A.
- After return,
G->A no longer contains (0,0), but G->nself_edges still reports 1.
- Subsequent calls that rely on cached state (e.g., algorithms that skip self-edge removal because
nself_edges == 0) produce wrong results or crash.
Impact
Silent, permanent graph corruption. The bug violates the caller's expectation that a quality-metric function is read-only. Cache inconsistency cascades to every subsequent operation on the graph object.
Suggested Fix
Work on a local copy of the adjacency matrix, exactly as LAGr_Modularity already does:
GrB_Matrix A = NULL;
GRB_TRY(GrB_Matrix_dup(&A, G->A));
// ... use A locally, free at end
Alternatively, document the mutation and fully invalidate all affected cached fields (nself_edges, degree vectors, etc.) before returning.
Summary
Severity: HIGH
Category: Input mutation / cached-state corruption
Location:
experimental/algorithm/LAGr_PartitionQuality.clines 88–91Trigger
Call
LAGr_PartitionQualityon any graph that has self-edges.Root Cause
GrB_Matrix A = G->Acreates a direct alias to the graph's adjacency matrix. Line 91 then calls:GrB_selectwith output == input performs an in-place operation, permanently deleting self-edges fromG->Aas a side effect of computing what is supposed to be a read-only quality metric. The cached fields (G->nself_edges,G->out_degree, etc.) are not invalidated afterward.Compare with
LAGr_Modularity, which correctly creates a local copy ofG->Abefore removing self-edges.Proof / Trace
Gwith self-loop(0,0)and cachedG->nself_edges = 1.LAGr_PartitionQualityis called; line 91 runsGrB_select(A, NULL, NULL, GrB_OFFDIAG, A, ...).(0,0)is permanently removed fromG->A.G->Ano longer contains(0,0), butG->nself_edgesstill reports1.nself_edges == 0) produce wrong results or crash.Impact
Silent, permanent graph corruption. The bug violates the caller's expectation that a quality-metric function is read-only. Cache inconsistency cascades to every subsequent operation on the graph object.
Suggested Fix
Work on a local copy of the adjacency matrix, exactly as
LAGr_Modularityalready does:Alternatively, document the mutation and fully invalidate all affected cached fields (
nself_edges, degree vectors, etc.) before returning.