Skip to content

fix: align raven-toolbox with RAVEN (parity review) - #182

Merged
edkerk merged 71 commits into
developfrom
parity/round-2
Oct 28, 2026
Merged

edkerk merged 71 commits into
developfrom
parity/round-2

Conversation

@edkerk

@edkerk edkerk commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Part of the behaviour parity review of raven-toolbox against RAVEN. Each change below was found by running the same input through both implementations with the scenarios of CompMetEng/raven-gecko-parity; the matching RAVEN changes are in CompMetEng/RAVEN#13.

  • reporter_metabolites: z-scores with rounding-level spread were not treated as no spread, the moments were not taken on centred z-scores, and p-values outside [0, 1] were dropped silently; they now raise ValueError.
  • fseof: the track used the signed flux and the knockout threshold differed; it regresses the absolute flux, reports every correlated track with a standard deviation above 0, and labels knockouts at 1e-8, as FSEOF.m does.
  • remove_duplicate_reactions: it grouped by stoichiometry only and had a reserved option; it groups by stoichiometry and reversibility, merges bounds and objective, and has no reserved option, as contractModel.
  • simplify_model: mode order, survivor choice, reserved reactions and orphan metabolites differed; they follow simplifyModel, and the zero-interval and no-flux modes keep genes.
  • prep_init_model: metabolites named by tasks were lost when the first simplification removed their last reaction, bounds were not reset after scaling, and non-text task ids were accepted; task metabolites are kept as in prepINITModel, unused genes are pruned, bounds are reset to ±1000, and non-text ids raise ValueError. Repeated task ids are accepted, failed tasks are dropped by position, and compare_models keeps tasks that share an id.
  • ftinit: the output was not built from the reference model with the template's bounds and directions, expression data was taken differently, and metabolites and genes without a reaction stayed in the output; all three follow ftINIT.
  • ftinit, run_ftinit: each scored reaction gets its own big-M, max(min(ub, big_m), min(-lb, big_m), force_on) with an infinite bound counted as big_m, as RAVEN's ftINITInternalAlg does; big_m is the cap on that capacity and defaults to 1000 (was a fixed 100). find_task_essential_reactions takes its candidates in the model's reaction order, so the essential set does not depend on the hash seed, and re-solves a knock-out from a cold start before counting it as infeasible, as solveLP does; on Human-GEM the essential set equals that of prepINITModel (204 reactions).
  • get_init_steps: the full series had no single-MILP run and passed mets_to_ignore; it carries RAVEN's single run and ignores no metabolites.
  • hpa_gene_scores, parse_hpa_rna: level names, tissue and cell type were compared case-sensitively, an unknown level was dropped, and the Sample and Value columns were read differently; they follow scoreModel and parseHPA, and an unknown level raises ValueError.
  • batch_curate, batch_curate_from_tsv: the rules of curateModelFromTables are applied: grouping, header names and aliases, plain decimal cells (no comma, hexadecimal or non-ASCII digit), cell trimming as MATLAB strtrim, empty cells overwrite existing annotations unless overwrite_with_empty=False, MIRIAM cells, whole-number charges, rev 0/1/true/false, unique rxnIdx, matching reaction tables, summed coefficient rows, gene rules with the grammar of parseGrRule, and every table is validated before the model is changed. Table files are read with the csv module so that a NUL character stays in its cell.
  • export_for_git: it supports all five formats, an exact txt table and fuller dependencies, main_branch_flag checks the raven_toolbox checkout, and its flags must be bool.
  • generate_new_ids, next_ids: the numeric maximum is taken and a suffix counts only if it is a finite whole number below 2^52.
  • set_gam, scale_biomass: the NGAM reaction is checked before the biomass reaction is changed, an NGAM value without a reaction raises ValueError, and the exact GAM value is stored.
  • write_yaml_model: empty reaction names were written, default bounds were written when not stored, and equation coefficients differed from num2str; they follow RAVEN, with a warning for duplicate metabolite names.
  • connect_blocked_reactions, fill_gaps_fast_lp, fill_gaps_milp: threshold, forced flux, net production, score handling and renamed template reactions follow fillGaps and gapFillMILP.
  • get_model_from_homology, load_uniprot, parse_task_list, write_fasta, expand_model, add_reactions_from_equations, rescale_for_init: small differences found by the scenarios are removed (complex policy and preferred order; whole words, isoform blocks and column order; an ID column in first position raises; an empty sequence gives the header only; the first isozyme copy keeps its position; new genes are added in sorted order; bounds are left alone).
  • sort_model: added, the counterpart of sortModel.
  • release.yml, parity-nightly.yml: pip is upgraded through python -m pip.

Calls that ran before and now raise ValueError: p-values outside [0, 1]; table cells that are not plain numbers, a fractional or infinite charge, a rev other than 0/1/true/false, a repeated or non-integer rxnIdx, a blank, NaN, zero or infinite coefficient, a malformed gene rule, a repeated gene id, and a new reaction that names an undefined gene; set_gam with a value and no reaction; export_for_git with a non-bool flag. batch_curate now overwrites existing annotations with empty cells by default.

Checks: the suite without tests/parity gives 1685 passed and 5 skipped with Gurobi, and 1676 passed and 9 skipped with GLPK; ruff and mypy are clean. The scenarios of CompMetEng/raven-gecko-parity#140 give 86 MATCH and 2 DIFFER on three seeds, and the two DIFFER are documented in its ledger.

Not checked: CI on GitHub, the tests marked parity on a machine without the full Gurobi licence, other cell lines and seeds of the Human-GEM-scale runs (on Human-GEM with HCT116 the ftinit result still differs from MATLAB ftINIT in the forced direction of reversible essential reactions in step 2), and curation tables from real projects against the stricter reading.

edkerk added 30 commits October 22, 2026 10:40
cobra's flux_variability_analysis and find_blocked_reactions start a worker pool by default. With Python 3.14 (forkserver start method) the pool did not return in gap_report on a cluster node: the main thread waited in Pool.join. The gap-filling modules and the localization certificate now pass processes=1, as the certificate's FVA already did.
…pendencies

The default formats are now yml, xml, mat, xlsx and txt, as in RAVEN. The txt table is headed 'Rxn id', writes bounds and objective coefficients with the digits needed to read them back (RAVEN's exportForGit does the same) and pads no field. dependencies.txt lists python, cobra, libSBML, ruamel.yaml, scipy, openpyxl and raven_toolbox in a fixed order, then the model's version and date; it holds no time of export.
The txt table and the Excel file wrote '1 ATP[c] => 1 ADP[c]' where RAVEN writes 'ATP[c] => ADP[c]'. A coefficient other than 1 is still written.
…oduction and score handling

A reaction is blocked when it cannot carry a flux above 1, the check is repeated with
the templates added, 0.1 is forced through each rescuable reaction, net production
relaxes the steady state in the checks as well as in the MILP, and scores that are not
negative take the least negative penalty.
A zero reference level scores 0/0 as -5 and a positive level as +10, a missing level
gives the gene no score, and a reaction whose genes have no score takes the no-gene
score instead of NaN.
The growth floor is one constraint on the objective, a template reaction with a positive
lower bound is only forced when it is selected, finite bounds are used as the coupling
constants, and a template reaction with the id of a draft reaction is renamed.
…order

Under flag an isozyme group without orthologs leaves one flagged subunit, under drop any
flagged subunit removes the reaction, a gene that maps to a gene already taken by an
earlier template is removed, and templates are merged in the order given.
…s and new ids

Coefficient rows are grouped by rxnIdx and name, RAVEN's header spellings are read,
new reactions without bounds are unbounded, and new ids use the width of the largest
existing id.
ftinit scores reactions from tissue, celltype, HPA levels and expression data as
scoreModel does; ready-made reaction scores stay available as rxn_scores.
…hen stored, warn on duplicate metabolite names
…reporter fold changes, task list comment column, test robustness
… and price renamed template reactions by their template id
…arately, new reactions take the stored defaultUB
…lites, renamed-id scores take precedence, defaultUB docstring
edkerk added 20 commits October 23, 2026 20:55
… prepINITModel does; duplicate task ids raise; batch_curate overwrite_with_empty

remove_zero_interval_reactions and remove_no_flux_reactions no longer remove genes, as removeReactions with its defaults. prep_init_model removes every gene without a reaction after the first simplification and raises ValueError for a task list with repeated ids. batch_curate and batch_curate_from_tsv take overwrite_with_empty (default True): an empty cell of an existing entity clears the stored value, False leaves it.
…res non-finite and fractional suffixes; export_for_git requires bool flags
…s take 0 for an empty charge or score, table files are read as text; overwrite_with_empty and flags are type-checked; documentation of next_ids
…ing matches scoreModel (first of case-only duplicate names, case-insensitive tissue and cell type, any unknown level raises)
…iven, text that is not a number refused); next_ids ignores suffixes of 2^52 or more
…eeps the first one per name that tasks name; batch_curate reads cells with RAVEN's plain decimal rule (no comma, hexadecimal or non-ASCII digit), validates rxnIdx and coefficient cells and trims only MATLAB whitespace; a non-finite charge raises ValueError; hpa blank level named in the error
…, true or false and an rxnIdx cell a whole number of at least 1; a coefficient row without content is ignored; ftinit removes every metabolite without a reaction from the extracted model
…s, tables are validated before the model is changed, a coefficients table without its coefficient column names the missing columns; tests
…er, validate every table before the model is touched, match reaction tables on the numeric index, sum repeated coefficients; ftinit drops genes without a reaction
…uched; row-number wording of the missing rxnIdx error follows RAVEN
… one reaction and a repeated new gene are checked before the model is touched; the rule of a new reaction names only known genes; core headers are matched as in RAVEN; the header row is read like readCurationTable
…that cannot be performed is removed by its position; compare_models fills the task matrix by position
… capped at big_m (default 1000)

The direction and off constraints of a scored reaction use max(min(ub, big_m),
min(-lb, big_m), force_on), with an infinite bound counted as big_m, as RAVEN's
ftINITInternalAlg does. big_m is the cap on that capacity.
…, so the essential set does not depend on the hash seed
…ing it infeasible

A knock-out solve starts from the basis of the previous one. From such a
start Gurobi reported two feasible Human-GEM knock-outs as infeasible
(MAR04410, MAR03802), which put them in the essential set. A status that
is not feasible is now checked again from a cold start, as RAVEN's
solveLP does after a failed hot-started solve, and only a knock-out that
is infeasible on both solves counts as essential. Feasible statuses
follow RAVEN's checkSolution (optimal, suboptimal, unbounded).
@edkerk
edkerk merged commit 73a88ec into develop Oct 28, 2026
10 checks passed
@edkerk
edkerk deleted the parity/round-2 branch October 28, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant