Repository navigation
Align task lists, GPR parsing and YAML warnings with RAVEN - #181
Merged
Merged
Conversation
…une ties parse_task_list: an empty CHANGED LB/UB now leaves the reaction bound as it is (None) instead of defaulting to -1000/1000, and .xls gets a clear error. check_tasks warns when a name[comp] matches several metabolites. is_dnf raises ValueError on a malformed rule instead of reporting True. write_yaml_model warns on metabolites with an empty name. Tests for each, plus ties in remove_low_score_genes.
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.
edkerk
added a commit
to CompMetEng/raven-gecko-parity
that referenced
this pull request
Oct 21, 2026
New scenarios: remove_low_score_genes_ties_smallyeast, task_list_edge_cases and yaml_float_digits_smallyeast. The task-list scenarios compare an empty CHANGED bound as NaN. Ledger notes for replaceMets, removeLowScoreGenes, parseTaskList, checkTasks, writeYAMLmodel, exportForGit and isDnfGrRule now say what CompMetEng/RAVEN#11 and CompMetEng/raven-toolbox#181 change.
edkerk
added a commit
to CompMetEng/raven-gecko-parity
that referenced
this pull request
Oct 21, 2026
…otes for the parity decisions (#139) * fix: name the solver of min_nr_fluxes_toy instead of requesting scip The MATLAB side called setRavenSolver('scip'), which RAVEN supports on Linux only if SCIP was compiled by hand, so the scenario ended in ERROR (no result from matlab) in every nightly. It now takes matlab_solver and python_solver from scenario.yml (gurobi, as in task_checking_smallyeast) and restores the previous RAVEN solver afterwards. * doc: record the differences confirmed on MATLAB in the ledger notes The first MATLAB batch on a cluster (RAVEN 79a7d8c, 2026-10-21) confirmed differences between RAVEN and raven-toolbox that the notes did not state or stated wrongly: replaceMets (a RAVEN bug, not replicated exactly), removeLowScoreGenes (the tie-break is alphabetical, not random), parseTaskList, checkTasks, writeYAMLmodel, exportForGit and isDnfGrRule. Only the notes of these seven rows change. * Add scenarios for the parity decisions and update the ledger notes New scenarios: remove_low_score_genes_ties_smallyeast, task_list_edge_cases and yaml_float_digits_smallyeast. The task-list scenarios compare an empty CHANGED bound as NaN. Ledger notes for replaceMets, removeLowScoreGenes, parseTaskList, checkTasks, writeYAMLmodel, exportForGit and isDnfGrRule now say what CompMetEng/RAVEN#11 and CompMetEng/raven-toolbox#181 change.
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.
What this changes
Decisions taken after the RAVEN vs raven-toolbox parity review. Each item has a test.
parse_task_list: an emptyCHANGED LBorCHANGED UBnow leaves that bound of the reaction as it is (stored asNone). Before, it defaulted to -1000 or 1000 and overwrote the model's own bound. A.xlsfile now gets a clear error that names.xlsxand.txt.check_tasks: warns when a task'sname[comp]matches several model metabolites. All of them are still constrained. RAVEN constrains only the first. Thetask_name_mapsdocstring wrongly said RAVEN constrains all, and is corrected.is_dnf: raisesValueErroron a malformed rule string (G1 and,(G1 or G2,()and so on). Before, cobra swallowed the syntax error and the empty GPR counted as DNF. RAVEN'sisDnfGrRuleerrors on these.write_yaml_model: warns when a metabolite has an empty name. RAVEN's writer refuses such a model.remove_low_score_genes: behaviour is unchanged. Tests pin the tie rule (the gene or complex listed first in the rule is kept), next to thetied_reactionstests from fix: four writer, task and gene-pruning problems from #178 #179. The docstring no longer says RAVEN picks at random.Also includes the Windows CI fix from #180 (
python -m pip install --upgrade pip), now merged withdevelop.RAVEN side
The matching RAVEN changes are in CompMetEng/RAVEN#11: first-in-rule on ties, flags
0/false/noread as false,;lists trimmed, headers matched ignoring case, emptyCHANGEDbounds kept as NaN, a duplicate-metabolite warning incheckTasks, the shortest round-trip float format inwriteYAMLmodel, and thereplaceMetsfix.Checks
MATLAB R2024b with GLPK on RAVEN#11 head
875e45band this branch at8fbaed4:tTasks7/7,tIO24/24,tManipulation85/85,tINIT31/31,tSyntax1/1, and the scenariosremove_low_score_genes_smallyeast,replace_metabolite_toy,task_checking_smallyeast,task_list_parsingandyaml_roundtrip_smallyeastMATCH for seeds 0, 1 and 2. Results are in CompMetEng/raven-gecko-parity, branchhpc-results/2026-10-21.After merging
developinto the branch:pytest(1212 passed, 18 skipped),ruffandmypypass locally.Not checked: the MATLAB run predates the merge of
develop, which only touched other files and tests.