fix: sampleCHRR returns NaN when the equality system is square - #696
Merged
Conversation
sampleCHRR builds Aeq = [S; rows fixing implicitly-determined reactions]
and takes a particular solution with `Aeq \ beq`. When the number of
metabolites plus the number of folded-in fixed reactions happens to equal
the number of reactions, Aeq is square; it is also rank-deficient, and
MATLAB's backslash returns NaN for that case rather than a least-squares
solution.
The NaNs propagate into the reduced polytope, and the failure surfaces
later and misleadingly as
sampleChebyshevCenter: LP infeasible - flux polytope has empty interior.
Use lsqminnorm, which returns the minimum-norm solution for a
rank-deficient system of any shape -- and is what the Python reference
implementation already does, so the two now agree on the polytope.
Reproduces on the small yeast model shipped with the documentation site
(52 metabolites, 53 reactions, ethIN fixed at 0 -> Aeq is 53x53 with
rank 44). With the fix, sampling returns 9 dimensions and no NaNs,
matching the Python implementation's 9.
Function test results300 tests 274 ✅ 1m 7s ⏱️ Results for commit ba36fc1. |
edkerk
added a commit
to edkerk/raven-docs
that referenced
this pull request
Aug 26, 2026
* Add user guide page 15: random sampling Covers a first seeded sample, what a distribution says that a range does not, the choice between interior sampling and random-objective vertices, conditioning on a state before sampling, and the loopless screen behind the objective list -- MATLAB and Python side by side. Two things the examples measured that are worth calling out. The loop pair from page 14 is sampled across nearly its whole 1000-unit range, so loops are not a rare corner of the space but most of it; and sampled growth averages under half the optimum, because near-uniform sampling almost never lands on a vertex. Both make the same point: a sampled mean describes the shape of the feasible space, not the organism. The MATLAB CHRR block is skipped for now. randomSampling(...,'chrr') fails on smallYeast with "flux polytope has empty interior" because sampleCHRR's equality system is square and rank-deficient there, so its particular solution is NaN -- SysBioChalmers/RAVEN#696. The documented output is captured from a patched RAVEN; unskip once the fix ships. * Print sampled statistics to a precision that survives the platform CI put FRDS2's sampled minimum at 18.8 where this machine had 19.0, and mean growth at 0.0497 against 0.0496. Same seed, same code: MATLAB's samplers take their nullspace basis from null(), hence from LAPACK, so the walk differs between Linux and Windows even though the distribution does not. The Python chain reproduces exactly across both. Print two decimals rather than four, and say all of this in 15.1 -- a reader who seeds a chain and compares numbers with a colleague on another machine should know which parts are the result.
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.
Problem
sampleCHRRfails on a model as small as the one shipped with the documentationsite (52 metabolites, 53 reactions):
The polytope is not empty. The error comes from NaNs created much earlier.
sampleCHRRfolds implicitly-fixed reactions into the equality system:On this model exactly one reaction is fixed (
ethIN, bounds[0 0]), soAeqis 53 x 53 — square — with rank 44. For a rank-deficient square system
MATLAB's backslash returns
NaN, where for a rectangular one it would return abasic solution. The NaNs flow into
A_full/b_full, and the failure onlysurfaces later, in
sampleChebyshevCenter, as an infeasible LP.The trigger is therefore
nMets + nFixed == nRxns, which is not exotic — it isone arithmetic coincidence away on any model.
Fix
lsqminnormreturns the minimum-norm solution for a rank-deficient system ofany shape. As the surrounding comment already notes,
v0may be any particularsolution — the sampled distribution does not depend on the choice — so this
changes nothing except the degenerate case. It also matches the Python
implementation, which uses
lstsq.Verification
On the same model, before: the error above at every
fixedWidthToltried(1e-7, 1e-6, 1e-4). After:
The Python implementation reports 9 dimensions and 1 fixed reaction for the
same model, so the two now agree on the geometry; the sampled growth
distribution matches too (0.0098..0.0911 there, a different RNG).
randomSampling's defaultachrpath is untouched and still returns NaN-freesamples.
Found while writing the sampling page of the new user guide.