Reconstruction: BLAST/DIAMOND wrappers and homology-based drafting#4
Merged
Conversation
edkerk
added a commit
that referenced
this pull request
Jun 20, 2026
Ran six tests on yeast-GEM (4102 rxns), iJO1366 (2583 rxns), e_coli_core (95 rxns), and the tINIT synthetic testModel. Two tests (#2 evalue, #5 threads) require BLAST/HMMER binaries not available in this environment. Results: - #1 replace_max_bound: Python False CORRECT; True causes unbounded solver on yeast-GEM (4083/4102 rxns at big-M → +inf opens unbounded objectives) - #2 evalue: untestable; leave at 1e-5 (matches blastp default) - #3 allow_excretion: effect is zero with default prod_weight=0.5; fix inconsistency between get_init_model (True) and run_init (False) - #4 flux_eps: Python 1e-6 CORRECT; 1e-8 picks up 21 false-positive targets with flux std ~5e-7 (solver noise, not biology) on iJO1366 - #5 threads: untestable; mark for performance fix without benchmark - #6 remove_genes blocked_reactions: Python 'remove' CORRECT; 'keep' gives wrong essentiality prediction for single-gene reactions on e_coli_core - #7 time_limit (localization): yeast-GEM solves in ~2.6 min; leave None, document 900s cap for Human-GEM scale - #8 mip_gap/time_limit (init): toy model too small to distinguish; leave None, document MATLAB's 0.0004/5s as guidance for genome-scale Three items confirmed correct and closed (#1, #4, #6). Three items need code changes (#3, #5, #7/#8 docstring only). Two items remain open (#2, #5).
edkerk
added a commit
that referenced
this pull request
Jul 1, 2026
* Add parameter defaults inventory and evaluation plan to maintenance docs * Evaluate parameter defaults: fill MATLAB RAVEN parity column, flag issues Compared every optional parameter against MATLAB RAVEN source and literature references. Eight issues identified: - High: replace_max_bound default mismatch (MATLAB True vs Python False) - High: BLAST/Diamond evalue mismatch (MATLAB 1e-4 vs Python 1e-5) - Medium: allow_excretion inconsistency (get_init_model vs run_init/run_ftinit) - Medium: flux_eps in FSEOF looser than MATLAB implicit tolerance - Medium: threads=1 throughout (MATLAB auto-detects cores) - Medium: remove_genes blocked_reactions behaviour inverted vs MATLAB - Low: predict_localization missing default time cap (MATLAB: 15 min) - Low: mip_gap/time_limit not defaulting to MATLAB's tuned values Status column updated from ? to ✓/⚠ for all evaluated parameters. * Reframe evaluation methodology: empirical testing over MATLAB parity MATLAB RAVEN defaults were never systematically validated. Change the methodology to treat MATLAB values as a prior, not ground truth: - Criterion 1 is now empirical testing on real models (iJO1366, Yeast9, Human-GEM), measuring result quality vs a benchmark - Criterion 2 is sensitivity envelope analysis - MATLAB cross-check is criterion 4 (corroborating evidence, not authority) - Evaluation workflow now requires running candidate values and reporting precision/recall or sensitivity profiles, not just citing MATLAB All "Proposed fix:" notes in the inventory replaced with "Open question:" framing that points to the specific test needed to settle the question. * Add empirical test results for all 8 parameter default questions Ran six tests on yeast-GEM (4102 rxns), iJO1366 (2583 rxns), e_coli_core (95 rxns), and the tINIT synthetic testModel. Two tests (#2 evalue, #5 threads) require BLAST/HMMER binaries not available in this environment. Results: - #1 replace_max_bound: Python False CORRECT; True causes unbounded solver on yeast-GEM (4083/4102 rxns at big-M → +inf opens unbounded objectives) - #2 evalue: untestable; leave at 1e-5 (matches blastp default) - #3 allow_excretion: effect is zero with default prod_weight=0.5; fix inconsistency between get_init_model (True) and run_init (False) - #4 flux_eps: Python 1e-6 CORRECT; 1e-8 picks up 21 false-positive targets with flux std ~5e-7 (solver noise, not biology) on iJO1366 - #5 threads: untestable; mark for performance fix without benchmark - #6 remove_genes blocked_reactions: Python 'remove' CORRECT; 'keep' gives wrong essentiality prediction for single-gene reactions on e_coli_core - #7 time_limit (localization): yeast-GEM solves in ~2.6 min; leave None, document 900s cap for Human-GEM scale - #8 mip_gap/time_limit (init): toy model too small to distinguish; leave None, document MATLAB's 0.0004/5s as guidance for genome-scale Three items confirmed correct and closed (#1, #4, #6). Three items need code changes (#3, #5, #7/#8 docstring only). Two items remain open (#2, #5).
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.
Generic BLAST and DIAMOND subprocess wrappers (DataFrame hits, no parallel struct) and the homology-based draft model builder ported from getModelFromHomology — with the structured improvements over RAVEN documented in IMPROVEMENTS.md (H1–H6).
Base: feature/io.