fix: gapFillTopological seed detection and zero-substrate reactions - #674
Merged
Conversation
gapFillTopological reported every metabolite as blocked on models fed
through RAVEN's own 'in' exchanges. Two independent defects, both hitting
the same reactions:
Seed detection tested lb < 0. That is correct under cobra's convention,
where analyse_topology reads it and every exchange is written "met <=>", but
addExchangeRxns writes an 'in' exchange as "=> met" with lb = 0. The test
therefore found no uptake at all, warned as much, and seeded nothing. Ask
getExchangeRxns instead of hand-rolling the rule. 'uptake' on its own is not
enough: it means the bounds allow *only* uptake, so a reversible exchange is
classified 'reverse' despite supplying the metabolite. The union of the two
is exactly "forward flux produces it, or reverse flux does".
Zero-substrate reactions never fired. subCountFwd is 0 for them, but firing
is only ever checked while walking subOf, and a reaction with no substrates
is in no metabolite's subOf list, so the countdown could not reach it. Fire
them before the loop, as _compute_scope does. This is how "=> met" exchanges
enter the scope.
Reproduced on A -> B -> C with addExchangeRxns(m,'in',{'A'}): reachableMets
was [0 0 0] and all three metabolites came back blocked; now [1 1 1] and
none. It escapes to the public API via fillGaps(...,'algorithm','topological').
Only the forward pre-fire is ported. _compute_scope does not pre-fire in
reverse either, and it needs no equivalent here: the reactions with no
products are exactly the "met =>" exchanges, which the seed rule above
already covers.
Tests: the existing case asserted only isfield, so it stayed green while the
function was inverted. Adds three behavioural tests covering 'in' exchanges,
reversible exchanges, and a no-exchange positive control.
Function test results243 tests 221 ✅ 1m 10s ⏱️ Results for commit 6131a83. |
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
1 participant
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.
Main improvements in this PR:
gapFillTopologicalreported reachable metabolites as blocked: seed detection missed'in'exchanges, and zero-substrate reactions never fired.tGapfilling.m.Instructions on merging this PR:
develop3as target branch, and will be resolved with a squash-merge.