KISS/DRY cleanup: hot-path micro-opts, dedup consolidation, CSP bench generators#16
Merged
Conversation
KISS/DRY: replace get_active_tensors (allocated a throwaway Vec per call) with a lazy active_tensors iterator; the three hot callers (occurrence scoring, NumUnfixedTensors, NumHardTensors) only iterate/count, so the Vec was pure waste. get_active_tensors kept as a thin .collect() wrapper for the one owned-slice caller (DRY). Behavior-preserving: node counts identical, 93 tests pass. Wall-clock ~neutral (~1-3%, within noise) — confirms the per-node cost is the region machinery, not these allocations. Also removes dangling PROF_* references left in examples/solve_circuit.rs by throwaway profiling instrumentation, which broke 'cargo build --examples' on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
…gion^2)) The growth loop rebuilt the frontier from scratch every step: a full scan of acc.vars x incident tensors with a fresh FxHashSet, plus a full re-score of every frontier tensor's shared-var count. Now the frontier is a persistent tid->shared-count map updated on each absorption (remove the absorbed tensor; each newly-added region var bumps its incident tensors' counts). Total frontier work drops from O(region^2 x degree) to O(region x degree). Bit-identical: the ranking sort key ends in the unique tid (a total order), so the unordered hashmap yields the same pick after sorting; the seed and maximality first-fit scan are unchanged. Node counts identical on factoring f16/f18 and X3C/QCP; 93 tests pass incl. the randomized region-vs-reference contraction check. Also threads the entailment memo through both memo helpers by argument (was a mutable capture) so they can share state. Wall-clock: ~2-4% (larger on big-region instances). NB the prior profiling that put this loop at ~51% of grow_region was timer-inflated on a tight loop; the join (contract.rs) is the real bulk. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
GreedyMerge re-evaluates the same candidate clause many times, and the per-node `MeasureScratch.cache` existed to absorb those duplicate propagations. ob-core now memoizes `size_reduction` by `(mask, val)` one level up (branch `greedymerge-memoize-size-reduction`), covering the measure too — so this downstream cache gets zero hits and is dead code. IPSolver/LPSolver/NaiveBranch evaluate each clause once and never needed it. Remove the cache field, its per-node clear, and the hit/miss + debug self-verification logic; `apply_branch` is now a thin stateless probe wrapper. Dedup now lives at the layer where the re-evaluation originates and caches the whole result, not just the propagated domains. Behavior unchanged: node counts bit-identical, 93 tests pass, ~2% faster on factoring (the memo's measure savings), more on larger instances. Point the ob-core dep at the memoization branch until it lands on main. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
Reproducible generators for decision families that probe where region contraction (structural, local inference) can beat flat CDCL: bounded-treewidth extensional CSP, XOR-SAT, Tseitin, coloring, quasigroup completion, exact-cover by 3-sets, plus an OR-Tools CP-SAT baseline that solves the native .csp relations directly. Retained as exploratory probes; the current paper direction is uniform cubing for parallel CnC, not a claimed CSP win. See README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
The greedymerge size_reduction memoization merged into OptimalBranching/optimal-branching main (#2, squash 51fb29d9), so drop the branch pin and track main; Cargo.lock pins the exact rev. Bit-identical to the pre-merge branch — verified by a 20x20 factoring differential run (branching nodes / visited / leaf all identical against un-memoized main). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
Format the incremental grow_region frontier code (ec9c09b) per rustfmt; fixes the CI Format check. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh
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
1 participant
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.
Consolidation pass — no algorithm changes, verified behavior-preserving.
Hot-path micro-opts
aa70d31alloc-freeactive_tensorsiterator; drop leaked profiling cruft.ec9c09bmaintain thegrow_regionfrontier incrementally — O(region) instead of O(region²).DRY: consolidate size_reduction dedup into ob-core
5efbfa6drop the dead per-nodeapply_branchcache inadapter.rs(MeasureScratch).8776d63trackoptimal-branchingmain, which now memoizessize_reductionat the source (greedymerge: memoize size_reduction, hoist constantbeforeoptimal-branching#2).The dedup used to live as a band-aid cache in bi's adapter, working around ob-core re-scoring the same clause. That memo now lives in ob-core's GreedyMerge where the redundancy originates, so bi's workaround is removed. Bit-identical: a 20×20 factoring differential run gives identical
branching_nodes/visited/ leaf counts (99142 / 158289) against the un-memoized upstream.Benchmarks
1c4375ereproducible CSP instance generators (bounded-treewidth extensional CSP, XOR-SAT, Tseitin, coloring, quasigroup completion, exact-cover-by-3-sets) + an OR-Tools CP-SAT baseline. Exploratory goal-D probes; seebenchmarks/csp/README.md.Tests: 93 unit + integration green; clippy clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DER3ZNDekiqXfmLH7XDVXh