feat: preconditioning tools (precond.py) with Jacobi and randomized Nyström - #27
Merged
Conversation
Introduce src/nncg/precond.py collecting the preconditioning tools, and generalise the CG inner solver to a callable preconditioner M^-1: v -> v. - precond.py: inverse_diagonal (Jacobi scaling, SPD-positivity guard), identity, diagonal, jacobi, and nystrom (randomized Nyström / Woodbury low-rank preconditioner, scalar-shift, O(n r) apply). The default shift is the largest uncaptured eigenvalue (deflation choice), overridable. - krylov.py: pcg now takes a callable preconditioner (None -> identity), so plain CG is identity-PCG; cg collapses to a thin wrapper over the single pcg loop. Add the Preconditioner type alias. - solver.py: inner="pcg" passes diagonal(dinv[F]); add inner="nystrom" that builds a fresh low-rank sketch per free set (cached across the p+1 RHS of an equality solve) with nystrom_rank/oversample/shift/seed knobs. - tests: new test_precond.py; nystrom inner-solver tests for solve_nnqp and solve_nnqp_eq (optimum recovery, trajectory invariance, eager guards). 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 introduces a dedicated preconditioning module and updates the Krylov inner solvers to accept callable preconditioners (r -> M⁻¹r), enabling non-diagonal options such as a randomized Nyström/Woodbury preconditioner. It then wires these options into the NNQP active-set solver and adds targeted tests for correctness and trajectory invariance.
Changes:
- Added
src/nncg/precond.pywith Jacobi/diagonal utilities plus a randomized Nyström preconditioner builder. - Generalized
pcgto accept a callable preconditioner and refactoredcgto delegate topcg(identity preconditioner). - Extended
solve_nnqp/solve_nnqp_eqwithinner="nystrom"and added tests validating recovery, guards, and free-set trajectory matching.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/nncg/precond.py |
New preconditioner builders (identity/diagonal/Jacobi/Nyström) as callables for PCG. |
src/nncg/krylov.py |
PCG now takes a callable preconditioner; CG is a wrapper over PCG with identity. |
src/nncg/solver.py |
Adds inner="nystrom" mode and threads Nyström parameters into free-set solves with per-free-set caching for eq solves. |
tests/test_precond.py |
New unit tests for preconditioner builders, guards, and Nyström behavior. |
tests/test_solver.py |
Adds NNQP tests for inner="nystrom" correctness and trajectory invariance. |
tests/test_eq.py |
Adds equality-constrained solve test covering Nyström path and cache behavior. |
tests/test_krylov.py |
Updates PCG tests to use diagonal(...) callable preconditioner rather than a raw vector. |
Comments suppressed due to low confidence (1)
src/nncg/krylov.py:116
- pcg() applies the preconditioner before checking for a zero RHS or an already-converged warm start. This does unnecessary work (and can call an expensive preconditioner even when the solve should return immediately). Consider moving the bnorm/initial residual checks before the first preconditioner application.
z = precond(r)
p = z.copy()
rz = float(r @ z)
bnorm = float(np.linalg.norm(rhs))
if bnorm == 0.0:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+94
| precond: The action ``r -> M^{-1} r`` of an SPD preconditioner. It is | ||
| applied once per iteration and must return a fresh vector. ``None`` | ||
| uses the identity preconditioner, so PCG reduces to plain CG. |
Comment on lines
+177
to
+180
| # Stabilising shift (Frangella-Tropp-Udell Alg. 2.1): lift Y off the range | ||
| # boundary so the small Cholesky is well conditioned, then subtract it back. | ||
| nu = np.sqrt(n) * np.finfo(np.float64).eps * float(np.linalg.norm(y, ord=2)) | ||
| y_nu = y + nu * omega |
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.
What
Adds
src/nncg/precond.pynext tokrylov.pyandsolver.py, collecting the package's preconditioning tools, and generalises the CG inner solver to a callable preconditionerM⁻¹: v → v.Why
Preconditioning was previously a single inline
dinv = 1.0 / op.diaginsolver.py. This factors it out and opens the door to non-diagonal preconditioners — specifically a randomized Nyström / Woodbury low-rank preconditioner, which cannot be expressed as adinvvector and needs the callable form.Changes
precond.py(new) — preconditioners as callablesr → M⁻¹r:inverse_diagonal(op)—1/diag(A)offop.diag, with an SPD-positivity guard (rejects non-positive/non-finite diagonals instead of leaking aninf).identity(),diagonal(dinv),jacobi(op).nystrom(matvec, n, rank, oversample=10, shift=None, seed=None)— randomized Nyström sketchA ≈ U diag(λ) Uᵀapplied as a scalar shift + low-rank Woodbury correction inO(nr), built once and captured in the closure. Default shift is the largest uncaptured eigenvalue (the deflation choice — measured 1.8–2.4× vs the smallest-captured alternative), overridable.krylov.pypcgtakes a callable preconditioner (None→ identity), applyingz = precond(r); added thePreconditionertype alias.cgcollapses to a thin wrapper over the singlepcgloop — plain CG is identity-PCG, removing the duplicated loop.solver.pyinner="pcg"passesdiagonal(dinv[F]); behavior unchanged.inner="nystrom"mode withnystrom_rank/nystrom_oversample/nystrom_shift/nystrom_seedknobs (seed fixed by default for reproducibility). Builds a fresh sketch per free set, cached across thep+1right-hand sides of an equality solve.Cost, stated plainly
Unlike Jacobi (a cached
diag(A)[F]slice), the Nyström sketch rebuilds per outer step — each free blockA_Fdiffers — costingrank + oversamplematrix-free products per free set on top of CG iterations. Worth it only when each free block's spectrum decays steeply. Documented in_make_free_solveand both public docstrings.Tests
New
tests/test_precond.py(guards,identity == cg, Jacobi, Nyström correctness / win / full-rank / error cases), plusinner="nystrom"tests forsolve_nnqpandsolve_nnqp_eq(optimum recovery, KKT certificate, same free-set trajectory as CG per the inexactness lemma, eager rank guard).All gates green: 85 tests pass,
ty+mypystrict clean,deptryclean,fmtclean. No new runtime dependencies.🤖 Generated with Claude Code