deps: Bump libcanon to 0.1.3 - #82
Merged
Merged
Conversation
The submodule sat at e15bef1, which predates the libcanon 0.1.3 release. The correctness fixes were already in at that point, but the chain-rebuild guard from libcanon #21 was not. That guard matters. Rebuilding the automorphism chain after conjugation, added in libcanon #18 to fix a wrong answer, costs 2.4 to 2.8 times the previous runtime on large single-node symmetry groups. The guard skips the rebuild when the ordering is already intact, which recovers that. The drudge test suite does not exercise a case large enough to show the difference: 162 passed, 2 skipped, in 5.6 seconds on either pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage Report for CI Build 31889963442Coverage remained the same at 87.887%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Drudge’s pinned deps/libcanon dependency to pick up libcanon v0.1.3 and the subsequent performance guard (DrudgeCAS/libcanon#21) that avoids unnecessary automorphism-chain rebuilds after conjugation, improving canonicalization performance on large symmetry groups.
Changes:
- Bump the
deps/libcanonsubmodule pin frome15bef1to7ed33f5(one commit past the libcanon v0.1.3 tag). - Pull in the chain-rebuild ordering guard so canonicalization can skip expensive rebuilds when ordering remains intact.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
deps/libcanonsubmodule sat ate15bef1, which predates the libcanon 0.1.3 release. All three wrong-answer canonicalization fixes were already in at that point, but the chain-rebuild guard from DrudgeCAS/libcanon#21 was not.That guard matters for performance. Rebuilding the automorphism chain after conjugation, added in DrudgeCAS/libcanon#18 to fix a wrong answer, costs 2.4 to 2.8 times the previous runtime on large single-node symmetry groups. The guard skips the rebuild when the ordering is already intact.
This moves the pin to
7ed33f5, libcanon master, one commit past the 0.1.3 tag.The drudge test suite does not exercise a case large enough to show the difference. It passes on either pin, 162 passed and 2 skipped, in 5.6 seconds both ways.