Repository navigation
fix: outputs and metabolite merging in replaceMets - #755
Merged
Merged
Conversation
replaceMets declared removedRxns and idxDuplRxns as outputs but never assigned them, so requesting them raised an error. They are now taken from contractModel. Adds a regression test.
Member
Author
|
Ran the unit tests locally (MATLAB R2024b, Windows): 64 passed. The two failures are solverTests/testSCIP (message mismatch in the SCIP solver check) and solverTests/testCobra (COBRA Toolbox is not installed on this machine). Both are in solverTests, unrelated to this change; the new replaceMetsOutputsTest passes. |
idxDuplRxns now holds the indices of removedRxns in the input model. Stoichiometries are summed when a reaction contains both the replaced and the replacement metabolite, replacement indices are remapped after metabolite deletion, and the replacement metabolite is never deleted when merging by name. Adds regression tests.
edkerk
changed the title
edkerk
changed the title
edkerk
changed the title
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 the bugs in
replaceMetsreported in discussion #754 and found while reviewing the function.replaceMetsoutputs:removedRxnsandidxDuplRxnswere never assigned, andidxDuplRxnswas the per-reaction cell array ofcontractModelinstead of the indices ofremovedRxnsin the input model.replaceMetsmetabolite merging: stoichiometries were overwritten instead of summed when a reaction contained both metabolites, the replacement metabolite could be deleted or counted twice when merging by name, and replacement indices were not updated after metabolite rows were deleted.replaceMetsmodel consistency:metNoteswas not removed together with the deleted metabolites, and the compartment lookup failed for a row-orientedcomps.replaceMetsedge cases: replacing a metabolite by itself deleted it, the verbose listing treated reaction identifiers as format strings, and duplicate reactions that already existed were removed even when no metabolite was deleted.The output, index, stoichiometry-summing and name-merging fixes have regression tests, and the HTML documentation is regenerated.