libsbml based SBML parser

[matthiaskoenig]

Hi all,

here a first version of the libsbml based SBML parser. This has the full feature set of the old parser and fixes a few smaller bugs I found on the way. All SBML based tests are passing.
The new parser is in the cobra.io.sbmlnew module, the tests are in test.test_io_sbmlnew. This is fully backwards compatible to the existing implementation (same function signatures).

I performed some timings and in the end the libsbml version is around 10-15% slower on reading very large models (RECON3D), and around 30-40% slower on writing very large models.

I did not look into any optimizations. Let me know what to change.
Best Matthias

[codecov-io]

Codecov Report

Merging #685 into devel will decrease coverage by 0.48%.
The diff coverage is 77.6%.

@@            Coverage Diff             @@
##            devel     #685      +/-   ##
==========================================
- Coverage   84.62%   84.14%   -0.49%     
==========================================
  Files          43       43              
  Lines        3909     4105     +196     
  Branches      892      960      +68     
==========================================
+ Hits         3308     3454     +146     
- Misses        401      422      +21     
- Partials      200      229      +29

Impacted Files	Coverage Δ
cobra/core/metabolite.py	70.58% <ø> (ø)	⬆️
cobra/core/gene.py	73.91% <100%> (+0.69%)	⬆️
cobra/manipulation/delete.py	81.37% <33.33%> (-1.46%)	⬇️
cobra/medium/boundary_types.py	95.65% <50%> (-4.35%)	⬇️
cobra/core/model.py	85.4% <55%> (-4.52%)	⬇️
cobra/io/sbml.py	79.25% <79.13%> (+13.25%)	⬆️
cobra/core/group.py	96.15% <96.15%> (ø)

---

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 a3f37ec...24c2b9a. Read the comment docs.

[Midnighter]

When we last talked your new parser implementation was still faster than both existing versions. Is this due to the parsing of GPR associations or where is the biggest performance problem?

[matthiaskoenig]

Biggest performance hit is gprs. Basically one goes from ast node in sbml to string representation back to ast node in cobra. This requires 2 rounds of astnode parsing. This will probably give another 10 to 20 percent performance by just directly translating astnode in each other. I did a bit of profiling and expect the libsbml parser being 15 percent slower. Because of the full parsing of all information overhead which is in this range. Creating the cobra objects and optlang objects seems to be the main time the parsers are needing. So i can bring it down within the 20% slower.

…

On Sun, Mar 25, 2018, 22:30 Moritz E. Beber ***@***.***> wrote: When we last talked your new parser implementation was still faster than both existing versions. Is this due to the parsing of GPR associations or where is the biggest performance problem? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#685 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA29uoe4VyrIWQhzz47k9lsxZAwx6hnmks5th_59gaJpZM4S6M7L> .

[Midnighter]

That sounds great. You could also check performance when you have symengine installed.

[cdiener]

Honestly none of the time differences look very critical to me. In absolute terms they look pretty small. For small models this does not matter and for large ones the differences are pretty minor. I would rather concentrate on the following things:

• make it pass flake8
• use numpy style for the docstrings
• the gene parsing is currently a recursion and that migh lead to RecursionLimit errors for large GPRs
• what happens with models in the old COBRA format (GPR in notes)? we will probbaly have to support that for SBML 2 models...

Cheers and thanks for all the work. Looks very good!

[matthiaskoenig]

All points addressed, all tests passing.
Test coverage of the sbml.py was increased by 14% (the reduce in codecov is due to the groups feature, which reduces coverage of the model.py).

[matthiaskoenig]

Hi all,
any reasons why this cannot me merged in devel?
This is

• completely backwards compatible
• fixes a ton of issues
• has higher test coverage than the original code
• supports additional features (like reading and writing groups)
• passes all tests and pep8
• all requested changes have been implemented

I want to use this code in production.
Best Matthias

[Midnighter]

any reasons why this cannot me merged in devel?

Lack of time 😞 sorry.