refactor: split stoichiometry consistency in 3 tests

[carrascomj]

• [ ] fix #701
• [x] description of feature/fix Split the stoichiometric consistency problematic tests into three and put a cap on the number of inconsistent stoichiometries to be found so that it does not take forever on large reconstructions.
• [x] tests added/passed
• [x] add an entry to the next release

[codecov[bot]]

Codecov Report

Merging #703 into develop will decrease coverage by 0.05%. The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           develop     #703      +/-   ##
===========================================
- Coverage    75.55%   75.49%   -0.06%     
===========================================
  Files           50       50              
  Lines         2982     2987       +5     
  Branches       516      517       +1     
===========================================
+ Hits          2253     2255       +2     
- Misses         634      636       +2     
- Partials        95       96       +1     

Impacted Files	Coverage Δ
src/memote/support/consistency.py	95.50% <50.00%> (-1.43%)	: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 a5e62e6...46139a0. Read the comment docs.

[carrascomj]

Need further investigation. The yeast-GEM model is still giving Errored in the report for Unconserved metabolites and Minimal Inconsistent Errors, while the tests look good (they report the proper AssertionError with their corresponding messages after running memote report snapshot). It may be something with the configuration or the types of the metrics.

[carrascomj]

Sorry for the delay, I've answered to your comments in the review.

[Midnighter]

What's your sense, do we need some more testing on different models? Should we include a default timeout to the command line interface?

[carrascomj]

Yes, I think going with an opt-out 10 or 30 timeout is going to improve user experience. It is very unlikely to have an optimization of 10 seconds so we can just include in the documentation how to disable or change it for special cases. Anyways, I would like to have the thing with the error sorted out before merging this. To reproduce it:

curl -L -O https://raw.githubusercontent.com/SysBioChalmers/yeast-GEM/devel/ModelFiles/xml/yeastGEM.xml
memote -v DEBUG report snapshot --exclusive "test_unconserved_metabolites" yeastGEM.xml

[Midnighter]

I'm currently running both test_unconserved_metabolites and test_inconsistent_min_stoichiometry for the yeastGEM model and I think, we need to limit the number of investigated metabolites in test_inconsistent_min_stoichiometry. Most results look like singletons and (with CPLEX) the MIPs are solved very quickly (less than a second) but due to the sheer number it still takes a long time.

[carrascomj]

I experience the same results with the yeast GEM. With the cap, it took some time but it was quite less than test for open bounds so I think it is bearable. Shall we include it here or do I open another PR after this one?

[Midnighter]

Please re-introduce the cap here. I also saw that many of the unconserved metabolites are external ones. This means, we probably fail to detect exchange reactions in the yeast model leading to the large number of unconserved metabolites. Something to look into but definitely not in this PR.