opencobra / memote

memote – the genome-scale metabolic model test suite
https://memote.readthedocs.io/
Apache License 2.0
128 stars 28 forks source link

fix: detect minimal uncoservable sets #688

Closed carrascomj closed 4 years ago

carrascomj commented 4 years ago
carrascomj commented 4 years ago

This notebook shows that sympy obtains the same results by default as in the Figure 1 of Gevorgyan et al., 2008.

codecov[bot] commented 4 years ago

Codecov Report

Merging #688 into develop will decrease coverage by 0.09%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #688      +/-   ##
===========================================
- Coverage    83.05%   82.96%   -0.10%     
===========================================
  Files           49       49              
  Lines         2650     2653       +3     
  Branches       433      435       +2     
===========================================
  Hits          2201     2201              
- Misses         357      360       +3     
  Partials        92       92              
Impacted Files Coverage Δ
src/memote/support/consistency.py 96.92% <100.00%> (+0.03%) :arrow_up:
src/memote/support/consistency_helpers.py 100.00% <100.00%> (ø)
src/memote/support/matrix.py 100.00% <100.00%> (ø)
src/memote/suite/cli/reports.py 75.55% <0.00%> (-2.23%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ba30160...87f567b. Read the comment docs.

BenjaSanchez commented 4 years ago

@carrascomj @Midnighter did you observe any effects on performance from these changes? I just upgraded my memote from 0.10.2 to 0.11.0 and now it seems to get permanently stuck in test_stoichiometric_consistency when running the test suite on yeastGEM:

memote report snapshot yeastGEM.xml --solver-timeout 30

Context

System Information ================== OS Windows OS-release 10 Python 3.7.7 Package Versions ================ Jinja2 2.11.2 click 7.1.1 click-configfile 0.2.3 click-log 0.3.2 cobra 0.18.1 cookiecutter 1.7.2 depinfo 1.5.3 equilibrator-api 0.1.26 future 0.18.2 gitpython 3.1.1 goodtables 2.4.12 importlib-resources 1.4.0 lxml 4.5.0 memote 0.11.0 numpydoc 0.9.2 pandas 1.0.3 pip 20.1.1 pylru 1.2.0 pytest 5.4.1 requests 2.23.0 ruamel.yaml 0.16.10 setuptools 47.3.1.post20200622 six 1.14.0 sqlalchemy 1.3.16 sympy 1.5.1 travis-encrypt 1.1.2 wheel 0.34.2
carrascomj commented 4 years ago

@BenjaSanchez Yes, test_stoichiometric_consistency now runs a MILP problem for every unconserved metabolite, so it could be considerably slower for big reconstructions. The solver-timeout parameter is applied to every MILP problem (so it should finish); I'll check if the solver configuration is being correctly propagated for that case.

BenjaSanchez commented 4 years ago

@carrascomj @Midnighter I did some more testing, and after installing symengine, test_stoichiometric_consistency takes ~45 seconds (without it it's over an hour), making the whole test suite in yeast-GEM take ~12 minutes. So feel free to disregard my comment :) but maybe symengine could be a recommended dependency of memote? It's seems to significantly help with several tests.

EDIT: The test does however error now, so maybe this is still something that could be worked on.