Add find_active_reactions.py to find active reactions quicker

[shjchan]

Add find_active_reactions.py and a test_find_active_reactions.py. Had some discussion with @ChristianLieven and @Midnighter before. Said it might be nice to add these functions for finding active reactions (primarily for finding reactions in loops) which solve an MILP or a few LPs instead of doing FVA. From tests on a few different models, comparing to using cobra.flux_analysis.find_blocked_reactions.py, it can be 3 - 5x faster. Would be good to suggest ways to streamline the code and improve speed. I have been using cobrapy from time to time but not very into its infrastructure. The test may need changes too.

[codecov-io]

Codecov Report

Merging #841 into devel will decrease coverage by 0.46%. The diff coverage is 75.86%.

@@            Coverage Diff             @@
##            devel     #841      +/-   ##
==========================================
- Coverage   84.39%   83.93%   -0.47%     
==========================================
  Files          47       48       +1     
  Lines        4205     4433     +228     
  Branches      978     1044      +66     
==========================================
+ Hits         3549     3721     +172     
- Misses        423      458      +35     
- Partials      233      254      +21

Impacted Files	Coverage Δ
cobra/flux_analysis/helpers.py	66.66% <100%> (+16.66%)	:arrow_up:
cobra/flux_analysis/loopless.py	78.43% <67.1%> (-11.7%)	:arrow_down:
cobra/flux_analysis/find_active_reactions.py	79.6% <79.6%> (ø)

---

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 9d1987c...67f2863. Read the comment docs.

[cdiener]

It seems that you describe a full loop-removal procedure in the paper. If that does not have the same drawbacks as CycleFreeFlux it may be worthwhile to switch to yours. Could you use it for loopless FVA as well?

[shjchan]

It seems that you describe a full loop-removal procedure in the paper. If that does not have the same drawbacks as CycleFreeFlux it may be worthwhile to switch to yours. Could you use it for loopless FVA as well?

From what I understand, CycleFreeFlux is a post-optimization loop-removal procedure, which does not guarantee optimality. I guess it is particularly true for the reactions in loops. The method in my paper guarantee optimality as the original loop-law constraints and usually with much less binary variables, thus faster. The functions here are only for the first step of the whole localized-loopless-FVA procedure, i.e., to find a minimal feasible null space. The rest of the implementation will need some significant coding, which I hope I can do some time later.

[shjchan]

Any idea why the test is stuck at test_fastcc.py? I remember that when I just made the pull request, it ran through it.

[shjchan]

I didn't have the time yet to go through the entire code. Out of curiosity, how is your algorithm affected by (how does it treat) reactions that are unbounded in one direction? Are they then also intended to be bounded by big M?

They are bounded by either big M (if relax_bounds=True) or the original bounds (if relax_bounds=False). The additional constraints for irreversible reactions are actually similar to fastcc while for the reversible reactions are a little different.

[shjchan]

It seems that you describe a full loop-removal procedure in the paper. If that does not have the same drawbacks as CycleFreeFlux it may be worthwhile to switch to yours. Could you use it for loopless FVA as well?

From what I understand, CycleFreeFlux is a post-optimization loop-removal procedure, which does not guarantee optimality. I guess it is particularly true for the reactions in loops. The method in my paper guarantee optimality as the original loop-law constraints and usually with much less binary variables, thus faster. The functions here are only for the first step of the whole localized-loopless-FVA procedure, i.e., to find a minimal feasible null space. The rest of the implementation will need some significant coding, which I hope I can do some time later.

Good news: the Fast-SNP which I also implemented in this pull request, can be readily used to largely reduce the time for loopless FVA already. I reorganized the newly added functions and added a method options to flux_variability_analysis (just a few lines). Tested on E. coli core model, results exactly the same. Tested on some reactions in iJO1366, around 40x time reduction (16xx sec --> 40 sec).

[cdiener]

Good news: the Fast-SNP which I also implemented in this pull request, can be readily used to largely reduce the time for loopless FVA already. I reorganized the newly added functions and added a method options to flux_variability_analysis (just a few lines). Tested on E. coli core model, results exactly the same. Tested on some reactions in iJO1366, around 40x time reduction (16xx sec --> 40 sec).

This sounds great. If I find the time I will give it a go with the problematic example from #698. If it handles that well I think the way to go would be to drop CycleFreeFlux in favor of your method.

[shjchan]

@cdiener great. I am pretty sure that it will work fine. By the way, if you have time, could you advise me what goes wrong in the test? It just froze after test_fastcc. I couldn't figure why. I don't think I have changed anything related to that. And I could finish the run if I ran pytest locally.

[shjchan]

I think I have addressed all comments and have restructured the code so that it is more pythonic.

[cdiener]

Sorry for the delay, just saw it and will go through it again. Will probably take me a little since it is a lot of code changes. From a quick look I don't see any changes to flux_variability_analysis as mentioned above and we will need more tests.