Conversation
Implement Dostál & Schöberl's MPRGP (Modified Proportioning with Reduced
Gradient Projections) as a standalone, matrix-free, factorisation-free solver
for the same min_{x>=0} 1/2 x'Ax - b'x the active-set loop targets.
- src/nncg/mprgp.py: MPRGP solver with MPRGPConfig/MPRGPResult and the pure
gradient primitives (free/chopped/reduced-free gradient, max feasible step).
Interleaves conjugate-gradient, expansion and proportioning steps under the
proportioning test; A enters only via SymmetricOperator.matvec, and ||A|| for
the fixed step bound alpha_bar in (0, 2/||A||] is estimated matrix-free by
power iteration (default 1/||A||, safe against the underestimate).
- src/nncg/api.py: solve_nnqp_mprgp one-call wrapper mirroring solve_nnqp.
- Export MPRGP, MPRGPConfig, MPRGPResult, solve_nnqp_mprgp from the package.
- tests/test_nncg/test_mprgp.py: 25 tests (recovery across kappa/seeds,
matrix-free Gram, agreement with ActiveSetSolver, warm start + projection,
max_iter contract, three-move bookkeeping, config validation, adversarial
termination, gradient primitives). 100% coverage on the new module.
Bound constraints only; the equality-augmented Bx=c variant needs a SMALBE/
SMALSE outer wrap and is out of scope here (use ActiveSetSolver.solve_eq).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A marimo notebook (04_mprgp_vs_active_set) that puts solve_nnqp and solve_nnqp_mprgp head to head on the shared planted-optimum problems: - confirms both reach the same certified minimiser; - an interactive live comparison (n / kappa / support / seed sliders) timing both at matched accuracy; - a conditioning sweep (wall-clock and matrix-vector products vs kappa) — the plot that answers "is MPRGP faster?": on dense SPD problems the active-set loop is as fast or faster, its lead widening with kappa; - MPRGP's CG/expansion/proportioning move breakdown across kappa; - MPRGP's first-order accuracy-vs-tolerance behaviour; - guidance on when to reach for each solver. Comparisons precompute alpha_bar (so MPRGP is not re-charged for its ||A|| estimate) and match accuracy explicitly, since MPRGP is first-order while the active-set loop returns near-exact iterates. Registered in the mkdocs nav. 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 adds a second bound-constrained solver to nncg: MPRGP (a matrix-free projection method) alongside the existing primal-dual active-set loop, plus tests and a documentation notebook comparing the two approaches.
Changes:
- Introduces
src/nncg/mprgp.pywithMPRGP,MPRGPConfig,MPRGPResult, and supporting gradient/step primitives. - Adds the convenience API entry point
solve_nnqp_mprgp, and exports the new solver fromnncg.__init__. - Adds a comprehensive test suite for MPRGP and a marimo notebook (plus mkdocs nav entry) comparing MPRGP vs the active-set solver.
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/mprgp.py |
New MPRGP solver implementation, config/result types, and core iteration logic. |
src/nncg/api.py |
Adds solve_nnqp_mprgp one-call wrapper and updates module docstring. |
src/nncg/__init__.py |
Re-exports the new solver types and wrapper function. |
tests/test_nncg/test_mprgp.py |
New tests covering solver behavior, contracts, and primitives. |
book/marimo/notebooks/04_mprgp_vs_active_set.py |
New notebook comparing solvers and discussing tradeoffs. |
mkdocs.yml |
Registers the new notebook in the docs navigation. |
CLAUDE.md |
Documents the new module in the project layout overview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+273
to
+282
| x = np.zeros_like(b) if x0 is None else np.maximum(np.asarray(x0, dtype=np.float64), 0.0) | ||
| g = matvec(x) - b # gradient A x - b | ||
| products = 1 | ||
| p = _free_gradient(x, g) # initial conjugate direction | ||
|
|
||
| stop = tol * (float(np.linalg.norm(b)) or 1.0) | ||
| cg_steps = expansion_steps = proportioning_steps = iterations = 0 | ||
| converged = False | ||
|
|
||
| while iterations < max_iter: |
Comment on lines
+155
to
+159
| def __post_init__(self) -> None: | ||
| """Validate that the proportioning constant is strictly positive.""" | ||
| if self.gamma <= 0.0: | ||
| msg = f"gamma must be strictly positive; got {self.gamma:.2e}" | ||
| raise ValueError(msg) |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Adds MPRGP (Modified Proportioning with Reduced Gradient Projections, Dostál & Schöberl 2005) as a second solver for the repo's core problem
min_{x≥0} ½xᵀAx − bᵀx, plus a marimo notebook comparing it head-to-head with the existing active-set loop.MPRGP is a matrix-free, factorisation-free projection method — a complement to
ActiveSetSolver, not a replacement. It stays feasible throughout and interleaves three moves under the proportioning test‖β‖² ≤ γ²·φ̃ᵀφ:The fixed step
ᾱ ∈ (0, 2/‖A‖]is the only tuning;‖A‖is estimated matrix-free by power iteration (safe default1/‖A‖, since power iteration approaches λ_max from below).Aenters only viaSymmetricOperator.matvec, so then×nmatrix is never formed.What's in it
src/nncg/mprgp.py—MPRGPsolver +MPRGPConfig/MPRGPResultand pure gradient primitives (free / chopped / reduced-free gradient, max feasible step). 100% covered.src/nncg/api.py—solve_nnqp_mprgpone-call wrapper mirroringsolve_nnqp.__init__.tests/test_nncg/test_mprgp.py— 25 tests (recovery across κ/seeds, matrix-free Gram, agreement withActiveSetSolver, warm start + projection,max_itercontract, three-move bookkeeping, config validation, adversarial termination, gradient primitives).book/marimo/notebooks/04_mprgp_vs_active_set.py— comparison notebook (agreement, live slider comparison, conditioning sweep, move breakdown, accuracy-vs-tolerance, guidance), registered in the mkdocs nav.CLAUDE.md— documented the new module.Scope
Bx = cvariant needs an augmented-Lagrangian outer wrap (SMALBE/SMALSE) and is out of scope; useActiveSetSolver.solve_eqfor equalities.kkt_violation/‖b‖ ≈ tolat all conditioning, and reaching1e-6inxat κ=1e5 needstol≈1e-11.Is it faster?
On dense SPD problems, no — the notebook's conditioning sweep shows the active-set loop is as fast or faster, its lead widening with κ (MPRGP takes √κ-growing full-size steps vs a few reduced-block outer steps). MPRGP's edge is elsewhere: huge sparse/matrix-free operators, combinatorially hard active-set searches, and problems wanting a proven convergence-rate bound.
Testing
make test— 127 passed, 100% coverage.make fmt/make typecheck(ty + mypy --strict) /make deptry— all clean.uv run) and viamarimo export html --sandbox.🤖 Generated with Claude Code