[coz3-deepperf-fix] Batch fixed-column bound-witness linearization per row in lar_solver#10029
Draft
levnach wants to merge 1 commit into
Draft
[coz3-deepperf-fix] Batch fixed-column bound-witness linearization per row in lar_solver#10029levnach wants to merge 1 commit into
levnach wants to merge 1 commit into
Conversation
explain_fixed_in_row explained each fixed column of a row independently via explain_fixed_column, re-traversing and re-inserting the shared bound-witness dependency sub-DAGs once per column. Collect the lower and upper witnesses of all fixed columns in the row and linearize them in a single u_dependency_manager pass, so each shared sub-DAG node and each distinct leaf constraint is handled once. explanation is a set, so the result is identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes LP conflict explanation generation by batching dependency linearization for all fixed columns in a row, avoiding repeated traversals of shared bound-witness dependency sub-DAGs. It adds a row-level explanation helper in lar_solver and updates the bound propagator to use it.
Changes:
- Add
lar_solver::explain_fixed_in_row(row, ex)that collects all lower/upper witnesses for fixed columns in a row and linearizes them in oneu_dependency_manager::linearize(ptr_vector<...>, ...)pass. - Update
lp_bound_propagatorrow-explanation helpers to delegate to the new batched method (while keepingexplain_fixed_columnfor single-column use). - Introduce a reusable temporary
ptr_vector<u_dependency>buffer (m_tmp_witnesses) inlar_solver’s implementation state.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/math/lp/lp_bound_propagator.h | Switch row-level fixed-column explanation to the new batched lar_solver API; keep base lookup logic intact. |
| src/math/lp/lar_solver.h | Declare the new explain_fixed_in_row API alongside explain_fixed_column. |
| src/math/lp/lar_solver.cpp | Implement batched witness collection + single-pass dependency linearization; add a temporary witness buffer in imp. |
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.
Summary
lp_bound_propagator::explain_fixed_in_rowexplained every fixed column of a rowindependently, calling
lar_solver::explain_fixed_columnonce per fixed column(
src/math/lp/lar_solver.cpp). Each such call linearizes the lower- andupper-bound witnesses of a single column — a BFS over the
u_dependencyDAG usingthe dependency manager's mark bits — and inserts every reached leaf constraint into
the
explanation.Fixed columns of the same row routinely share large portions of their bound-witness
sub-DAGs (common ancestor constraints). The per-column scheme therefore re-traverses
those shared sub-DAGs and re-inserts their leaves once for every column, with an
independent mark/unmark cycle per column.
Change
Add
lar_solver::explain_fixed_in_row(row, ex), which collects the lower/upperwitnesses of all fixed columns in the row and linearizes them together in a single
u_dependency_manager::linearizepass.lp_bound_propagator::explain_fixed_in_rowand
explain_fixed_in_row_and_get_basenow delegate to it; the base-column lookup inthe latter is unchanged.
explain_fixed_columnis kept for its single-column caller.Why it is correct
explanationis a set —push_backdeduplicates. Dependency reachability ismonotone, so the union of the per-column leaf sets equals the leaf set of the union
of all roots: the batched pass yields exactly the same explanation. The manager's
mark bits guarantee each shared sub-DAG node is visited once, and the
linearize(ptr_vector, ...)overload already skips null/duplicate roots.Complexity
For a row with
Nfixed columns:O(Σ_j |witness-DAG(j)|)traversal +O(Σ_j leaves(j))set insertions, withNmark/unmark cycles;O(|⋃_j witness-DAG(j)|)traversal +O(#distinct leaves)set insertions, with a single mark/unmark cycle.Shared sub-DAGs are walked and their leaves inserted once instead of once per column.
Measured effect
Profiled with callgrind on a representative conflict-heavy
QF_SLIAinput(
model_validate=true, bounded run), baseline vs. patched:lp::lar_solver::explain_fixed_columnon the hot path:24,337,671,616 → 0retired instructions (59.2% → 0% of the run), replaced by the single batched
traversal;
41,113,093,210 → 35,983,363,256(×0.875, ≈ 12.5%fewer) — the net work removed by de-duplicating shared sub-DAGs;
6.428 s → 6.079 s(≈ 5.4% faster);