Repository navigation
Make recommend() 2.8-4x faster in pure Python with identical recipes - #60
Merged
Merged
Conversation
…trix (#59) Profiling recommend() over the 52 benchmark cases showed NNLS at 97% of the time: about 44 solves per call, each rebuilding a Householder factorisation over every row (including one ridge row per product) about 8-13 times. - _simplify warm-starts each trial refit from the current solution without the removed product. Lawson-Hanson from a feasible start reaches the same optimum in fewer steps. Ceiling refits keep the cold start: warm-starting them reached the limit along a different weight path (valid, but not the same trade-off). - Passive-set least squares use Cholesky on a lazily cached Gram matrix with two refinement steps (corrected semi-normal equations), falling back to Householder when a pivot shows poor conditioning, so rank decisions on ill-conditioned problems are unchanged. Mean recommend() time falls from 239 to about 136 ms (default ammonium limit) and from 247 to about 77 ms with no ceilings. All 52 recipes are unchanged (largest final concentration change 3e-11 mg/L, identical product sets).
#59) Dual weights A^T(b - Ax) now come from cached Gram entries over the passive columns instead of a full matrix pass each iteration; Gram solutions are refined only when a Cholesky pivot shows weak conditioning; the unmet-target diagnosis computes each drop-one fit once instead of once per missed target; the unused _matvec is removed. Best of three alternating runs over the 52 benchmark cases against main: 248 -> 89 ms (default), 297 -> 95 ms (Cl ceiling), 146 -> 36 ms (no ceilings). Recipes are identical (largest final change 3e-11 mg/L).
20 tasks done
rafatahmed
added a commit
that referenced
this pull request
Nov 7, 2026
Make recommend() 2.8-4x faster in pure Python with identical recipes
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.
Fixes #59 within the owner's constraint: pure Python, zero runtime dependencies.
Root cause (measured)
NNLS was 97% of
recommend()time: about 44 solves per call (simplification trials, ceiling refits, miss diagnosis). Each solve rebuilt a Householder least-squares factorisation over every row, including one ridge row per product, about 8-13 times, and recomputed the dual weights over the full matrix every iteration.Changes (all pure Python)
Evidence (52 benchmark cases, best of three alternating runs, same machine)
mainRecipes are identical: across 156 recipe runs (3 configurations x 52 cases), no worse fit and no product-set change; the largest final-concentration change is 3e-11 mg/L. 357 tests pass (1 skipped), including the #36 near-dependent and scale tests.
Note for #40
The earlier 7.3 ms Horticalc figure is one compiled SciPy NNLS fit; FlahaX's
recommend()is a pipeline (fit, simplify, recover, ceilings, diagnosis). #40 will time single fit against single fit and pipeline against pipeline.