Repository navigation
fix: keep everything of every model in merge_models; name the subsystem of transport reactions - #180
Merged
Merged
Conversation
…em of transport reactions merge_models added only the metabolites and genes that a reaction uses, dropped the objective, gave a compartment name to the last model that named it, and ordered new metabolites and genes by the reactions that reached them. RAVEN's mergeModels keeps all metabolites, genes and compartments of every model, concatenates the objective, keeps the first model's compartment name, and appends new items in the order of their source model. Confirmed on MATLAB (RAVEN 79a7d8c): an unused metabolite and gene survive and both objectives are kept. add_transport_reactions left the subsystem empty. RAVEN's addTransport sets "Transport between <from> and <to>" (reversible) or "Transport from <from> to <to>" (irreversible), using the compartment ids; confirmed on MATLAB for c and m.
The test passed -I, which makes Python ignore PYTHONHASHSEED, and replaced the whole environment, which stops Python from starting on Windows (no SYSTEMROOT). It now keeps the environment and sets the seed.
pip 26.2.2 refuses to replace itself on Windows when run as plain pip and exits 1, so every Windows job failed before running any test.
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.
Two differences from RAVEN found by a review of the RAVEN and raven-toolbox pairs and confirmed by running the MATLAB side (RAVEN 79a7d8c, R2024b) on a cluster. Related: CompMetEng/raven-gecko-parity#139 (ledger notes) and the review in that repository.
manipulation.merge_models: only metabolites and genes that a reaction uses were added, and the objective was dropped. RAVEN'smergeModelskeeps every metabolite and gene of every model and concatenates the objective. On MATLAB an unused metabolite and gene survive and both objectives are kept (53 metabolites, 62 genes, two non-zero objective entries for two copies of smallYeast with one extra of each). The objective direction now comes from the first model.manipulation.merge_models: a compartment named in several models took the last model's name (RAVEN: the first). Compartments without metabolites were not carried over (RAVEN keeps them).manipulation.merge_models: new metabolites and genes were added in the order in which reactions reached them. Genes came out in a different order under differentPYTHONHASHSEEDvalues, because cobra builds the genes of a rule from a set. They are now added in the order of their source model, before the reactions; a test runs the merge under three hash seeds and requires identical output.manipulation.add_transport_reactions: the subsystem of the new reactions was empty. RAVEN'saddTransportsetsTransport between <from> and <to>(reversible) orTransport from <from> to <to>(irreversible), using the compartment ids; on MATLAB, a transport betweencandmgetsTransport between c and m.Each fix has tests: 13 new tests, 11 of which fail on the previous code and pass now; the other two guard behaviour that was already right. Full suite: 1166 passed, 9 skipped; ruff and mypy are clean for the changed files.
Not checked: the genome-scale parity tests (
pytest -m parity) need a full Gurobi licence and were not run. The MATLAB side was run for the behaviour of RAVEN, not for the new Python code.