Skip to content

graph/topo: add transitive reduction for DAGs#2070

Merged
kortschak merged 18 commits intogonum:masterfrom
Schwarf:feature/add_transitive_reduction
Feb 22, 2026
Merged

graph/topo: add transitive reduction for DAGs#2070
kortschak merged 18 commits intogonum:masterfrom
Schwarf:feature/add_transitive_reduction

Conversation

@Schwarf
Copy link
Copy Markdown
Contributor

@Schwarf Schwarf commented Jan 18, 2026

This PR implements transitive reduction for directed acyclic graphs (#1978).

Implementation notes

  • The algorithm removes redundant edges u→x whenever an alternative path u→v→…→x exists, preserving reachability by construction.
  • It processes each node’s successors incrementally and performs pruned DFS from each successor to detect transitive edges.
  • To scale to larger and denser DAGs, the implementation uses:
    • Dense node indexing with generation counters to track reachability and visited state without repeated clearing.
    • Reused scratch buffers to minimize allocations and GC pressure.

Result

  • Provides a single, optimized implementation.
  • Benchmarks show reduced runtime, memory usage, and allocations relative to an earlier implementation developed during this work (commit 4a33fa6), especially for dense graphs.

Fixes #1978

@Schwarf Schwarf force-pushed the feature/add_transitive_reduction branch from b48bfbc to 27a7220 Compare January 18, 2026 09:33
@Schwarf Schwarf force-pushed the feature/add_transitive_reduction branch from 27a7220 to 2ecabdb Compare January 18, 2026 09:45
@Schwarf
Copy link
Copy Markdown
Contributor Author

Schwarf commented Jan 18, 2026

“CI failed in stat/...: TestImportance (covariance matrix mismatch). I can’t reproduce locally; rerunning CI.”

@Schwarf
Copy link
Copy Markdown
Contributor Author

Schwarf commented Jan 31, 2026

Hi @kortschak,
just a gentle ping in case this slipped through — no rush.
This PR addresses #1978. Happy to make any changes if useful.

Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
@Schwarf Schwarf force-pushed the feature/add_transitive_reduction branch from 391dda7 to 516e30e Compare February 1, 2026 13:15
@Schwarf Schwarf requested a review from kortschak February 1, 2026 13:54
@Schwarf
Copy link
Copy Markdown
Contributor Author

Schwarf commented Feb 15, 2026

Hi @kortschak,
just a gentle ping in case this slipped off the radar 🙂
I updated the PR based on your feedback two weeks ago.
No rush at all—happy to adjust anything further if needed.

Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction.go Outdated
Comment thread graph/topo/transitive_reduction_bench_test.go
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go Outdated
@Schwarf Schwarf force-pushed the feature/add_transitive_reduction branch from 1378570 to 598379f Compare February 21, 2026 19:27
Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment thread graph/topo/transitive_reduction_test.go
@Schwarf Schwarf requested a review from kortschak February 21, 2026 19:37
Comment thread graph/topo/transitive_reduction.go Outdated
@Schwarf Schwarf requested a review from kortschak February 21, 2026 21:09
Copy link
Copy Markdown
Member

@kortschak kortschak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor issue only, then LGTM.

Comment thread graph/topo/transitive_reduction_test.go Outdated
},
check: func(t *testing.T, before, after graph.Directed) {
if after.HasEdgeFromTo(1, 4) {
t.Fatalf("expected redundant edge 1->4 to be removed")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Fatalf("expected redundant edge 1->4 to be removed")
t.Errorf("expected redundant edge 1->4 to be removed")

Also for the check functions below. This is explained in https://go.dev/wiki/TestComments#keep-going.

Comment thread graph/topo/transitive_reduction_test.go Outdated
Comment on lines +93 to +96
t.Fatalf("expected redundant edge 0->2 to be removed")
}
if !after.HasEdgeFromTo(10, 11) {
t.Fatalf("expected edge 10->11 to remain")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Fatalf("expected redundant edge 0->2 to be removed")
}
if !after.HasEdgeFromTo(10, 11) {
t.Fatalf("expected edge 10->11 to remain")
t.Errorf("expected redundant edge 0->2 to be removed")
}
if !after.HasEdgeFromTo(10, 11) {
t.Errorf("expected edge 10->11 to remain")

(these are why)

@Schwarf Schwarf requested a review from kortschak February 22, 2026 07:45
Copy link
Copy Markdown
Member

@kortschak kortschak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@kortschak kortschak merged commit 31a7a45 into gonum:master Feb 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature request: transitive reduction of a graph

2 participants