Repository navigation
Fix NNLS early stopping; add chloride salts and incidental-ion ceilings - #51
Merged
Merged
Conversation
Add Potassium Chloride and Calcium Chloride (Dihydrate) with identity-theoretical assays sourced to Sigma-Aldrich P3911 and 223506. Calcium chloride is the first calcium source without nitrate (#39). Chloride needs consent: recovery never admits it, so no default recipe changes. Record the ChemicalBook data sheets as sources for Ammonium Sulfate and Urea; their assays are unchanged. recommend() and plan_fertilizer_workflow() now use maximum_concentrations in the fit: an ion with a ceiling is admitted, is not penalised up to it, and is pulled back to it when exceeded. Any remaining excess is reported as a limit violation. Fix chloride in catalogue chemistry conversion (assay dropped, and calcium assumed to carry nitrate) and use 35.45 g/mol for chlorine everywhere.
The optimality test and the rank checks used a fixed 1e-10 threshold, so behaviour depended on problem scale. Near-dependent columns stopped before the optimum (Benchmark v1 near_dependent: objective 1.5e-12 instead of ~0), and a problem scaled by 1e-8 returned all zeros. The optimality test is now at round-off level, scaled by |A| and |b|, rank checks are relative to the largest column norm, and a column rejected as dependent is skipped until the solution changes, as in Lawson-Hanson. All 1,266 NNLS problems solved for the 52 benchmark recipes match SciPy as before, and no recipe dose changes.
This was referenced Oct 20, 2026
rafatahmed
added a commit
that referenced
this pull request
Nov 7, 2026
Fix NNLS early stopping; add chloride salts and incidental-ion ceilings
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.
Two commits from the root-cause audit of the open benchmark issues (tracker #42).
1. NNLS early stopping (#36)
Root cause: every threshold in
nnls/_lstsqwas a fixed1e-10, so behaviour depended on problem scale. Near-dependent columns stopped early (Benchmark v1near_dependent: objective 1.5e-12 instead of ~0), and a problem scaled by 1e-8 returned all zeros.Fix:
Evidence:
recommend()solves for the 52 benchmark recipes match SciPy before and after the change.2. Chloride salts, data-sheet sources and incidental-ion ceilings (#39)
Root cause of 10 recipe misses: Calcium Nitrate was the only calcium product, so Ca and NO3 came in a fixed ratio. A linear-programming check showed 16 of the 23 failing recipes have no solution within 1% from the old catalogue.
Catalogue (28 → 30):
Engine:
recommend(..., maximum_concentrations={"Cl": ...}), also passed through by the workflow.main.Fixes:
Not covered
Verification
sphinx -Wbuilds cleanly.Closes #36. Partly addresses #39: the remaining misses are coupled Mg/S targets and Pepper's multi-target conflict.