fix: honor warm start across all inner solvers (#24) - #25
Merged
Conversation
The warm= start was only partially wired: pcg had no x0 parameter, so the inner-solve seed never reached it, and the docstrings claimed only inner="cg" consumed the warm start at all. The outer warm start (starting from the prior free set) in fact already applied to every inner solver — only the inner-solve seeding was cg-only. - krylov.py: add an x0 warm start to pcg, mirroring cg (initial residual b - A x0); return zeros for a zero rhs regardless of x0. - solver.py: thread x0 into the pcg branch of _make_free_solve; rewrite the warm docstrings — cg and pcg both consume the v0 inner seed, exact is direct with nothing to seed, and all three start from the warm free set (single outer step on a support-stable step). Document why the v1 columns stay cold (their rhs is B_F, unrelated to x_prev). - tests: prove pcg honors x0 (test_krylov, test_solver); parametrize the eq support-stable single-step test over cg/pcg; pin the intentional "exact has no inner seed" behaviour with a dedicated eq test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes warm-start support across inner solvers by adding x0 seeding to pcg and threading it through the NNQP solver paths, while updating docstrings and tests to reflect the intended warm-start semantics for cg, pcg, and exact.
Changes:
- Add an
x0warm-start parameter topcg(analogous tocg) and propagate it through_make_free_solve. - Clarify warm-start behavior in
solve_nnqp/solve_nnqp_eqdocstrings (outer free-set warm start applies to all inners; inner seeding applies tocg/pcg, notexact). - Expand tests to assert
pcgiteration reductions from warm-starting and to cover equality-solver warm-start behavior acrosscg/pcgand the intentionalexactsemantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/nncg/krylov.py |
Adds x0 warm start handling to pcg and adjusts zero-RHS behavior. |
src/nncg/solver.py |
Threads x0 into the pcg free-block solve path and updates warm-start docstrings. |
tests/test_krylov.py |
Adds a test asserting warm-start reduces PCG iterations. |
tests/test_eq.py |
Parametrizes warm-start support-stable test over cg/pcg; adds exact warm-start outer-step test. |
tests/test_solver.py |
Adds bound-only solver test that warm-start reduces PCG inner iterations and yields one outer step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+105
to
+116
| if x0 is None: | ||
| x = np.zeros_like(rhs) | ||
| r = rhs.copy() | ||
| else: | ||
| x = x0.astype(np.float64, copy=True) | ||
| r = rhs - matvec(x) | ||
| z = dinv * r | ||
| p = z.copy() | ||
| rz = float(r @ z) | ||
| bnorm = float(np.linalg.norm(rhs)) | ||
| if bnorm == 0.0: | ||
| return x, 0 | ||
| return np.zeros_like(rhs), 0 |
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.
Closes #24.
Problem
solve_nnqp/solve_nnqp_eqaccept awarm=(free_mask, x_prev)tuple, but it was only partially wired.pcghad nox0parameter, so the inner-solve warm seed never reached it, and the docstrings overstated the gap — claiming onlyinner="cg"consumed the warm start.In fact the outer warm start (starting the active-set loop from the prior free set) already applied to every inner solver — that is what yields the single-outer-step property on a support-stable step. Only the inner-solve seeding was
cg-only.Change
krylov.py— add anx0warm start topcg, mirroringcg(initial residualb - A x0); return zeros for a zero rhs regardless ofx0.solver.py— threadx0into thepcgbranch of_make_free_solve; rewrite thewarmdocstrings so they are precise: cg and pcg both consume thev0inner seed, exact is a direct solve with nothing to seed, and all three start from the warm free set (single outer step on a support-stable step). Also document why thev1columns stay cold — their right-hand sides are the rows ofB_F, unrelated tox_prev.test_krylov.py::test_pcg_warm_start_reduces_iterations— direct proofpcghonorsx0.test_eq.py— parametrized the support-stable single-step test overcg/pcg; addedtest_eq_warm_start_exact_single_outer_steppinning the intentional "exact has no inner seed" behaviour (outer == 1from the warm free set alone).test_solver.py::test_warm_start_pcg_reduces_inner_iterations— covers the newpcgx0path in the bound-only solver.Acceptance criteria (from #24)
solve_nnqp_eqstep terminates in a single outer step — now asserted forcgandpcg.pcgnow honors the warm start;exact's ignore is documented-as-intentional with a dedicated test.Verification
make test→ 67 passed, 100% coverage.make fmtandmake typecheck(ty + mypy --strict) both clean.🤖 Generated with Claude Code