sys-bio / tellurium

Python Environment for Modeling and Simulating Biological Systems
http://tellurium.analogmachine.org/
Apache License 2.0
106 stars 36 forks source link

import libsbml results in error #475

Open hsauro opened 4 years ago

hsauro commented 4 years ago

I don't know if this has been reported before but I noticed that loading libsbml gives the following error, is it because I have an old libsbml installed?

import libsbml Traceback (most recent call last):

File "", line 1, in import libsbml

File "C:\Users\hsauro\AppData\Roaming\Python\Python37\site-packages\libsbml__init__.py", line 94285, in class DynPkgNamespaces(SBMLNamespaces):

File "C:\Users\hsauro\AppData\Roaming\Python\Python37\site-packages\libsbml__init.py", line 94346, in DynPkgNamespaces swig_destroy__ = _libsbml.delete_DynPkgNamespaces

AttributeError: module '_libsbml' has no attribute 'delete_DynPkgNamespaces'

luciansmith commented 4 years ago

libsbml doesn't officially come with tellurium--you have to use 'tesbml' instead. But it still uses the 'libsbml' directory, so it might get confused?

The actual solution is to just move to using the official libsbml release instead of the tesbml thing; I'll be looking into how best to do that. I know the various Frank-supported packages are basically ready.

fbergmann commented 4 years ago

yeah ... i think it would be best to not have the confusion about hte two packages. it would be fine for tellurium to depend on python-libsbml-experimental. Packages are up at pypi and conda-forge, so it should be no problem for you to switch.

matthiaskoenig commented 4 years ago

@Frank Bergmann frank.thomas.bergmann@gmail.com The reason why there is a separate package are the problems with using libsbml with sedml and other combine archives. This is basically the issue I discussed with you during Harmony (the overwriting of symbols). Kyle build libsbml and sedml with namespaces on, to avoid the clashing of symbols. To solve the libsbml/libsedml/libcombine ... symbol issue with swig would allow to use libsbml directly and simply things.

hsauro commented 4 years ago

This is a huge issue that been bugging me for some years, I wish users could just type

import libsbml, instead they have to do import tesbml

fbergmann commented 4 years ago

@matthiaskoenig , i am aware of that ... but as i demonstrated at HARMONY, even the tesbml/sedml/numl/combine packages overwrite each others symbols. It is just the way the SWIG wrappers are generated. Thus i was proposing of minimizing the additional work done.

luciansmith commented 4 years ago

Is the 'overwriting symbols' problem written up anywhere? And Frank, are you saying that creating 'tesbml' doesn't actually solve the problem?

When I asked Kyle the reasoning behind the tesbml thing, he just said it was to ensure versioning, not to solve any symbol overwriting issue. (And of course, there's the separate issue of Python dumping all dlls into the same place, meaning you can't have two different libX.dll files. I believe this problem was solved through making everything static, however.) I'd like to understand what the issues are here before trying to make a change one way or the other.

fbergmann commented 4 years ago

Have a look at this issue for a worked example and workaround:

https://github.com/fbergmann/libSEDML/issues/21

the same issue does apply to tesbml as well.

I'm not sure how versioning applies here. I mean you have just one requirements.txt file in which you prescribe what version of the packages you want for a specific version of tellurium. Having the packages duplicated for that does seem like a strain on resources. But maybe i don't quite understand the issue yet. In general if there are issues with interoperability of the libraries, i would prefer that an issue is filed for the main packages, so that they can be resolved, rather than the project forked over and over again.

matthiaskoenig commented 4 years ago

@Lucian Yes, as I understand the tesbml does not solve the problem (it just hides it from the user). Certain Objects will miss from the libraries, namely the Objects which are overwritten by both libraries (same for other Swig Python COMBINE libraries such as libnuml, libcombine, ... The only thing which is solved via building tesbml is that the python interpreter is not dying on imports and usage of the libraries. Whereas using the native libraries you will get segfaults with dying python interpreter. More specifically ff you use libsbml and sedml python bindings via the native pip packages the interpreter will die if you use objects which are defined in both libraries (e.g. MathML, ...). This depends on the import order of the libraries, with the last import overwriting the symbols.

On Wed, Sep 16, 2020 at 9:02 AM Frank Bergmann notifications@github.com wrote:

Have a look at this issue for a worked example and workaround:

fbergmann/libSEDML#21 https://github.com/fbergmann/libSEDML/issues/21

the same issue does apply to tesbml as well.

I'm not sure how versioning applies here. I mean you have just one requirements.txt file in which you prescribe what version of the packages you want for a specific version of tellurium. Having the packages duplicated for that does seem like a strain on resources. But maybe i don't quite understand the issue yet. In general if there are issues with interoperability of the libraries, i would prefer that an issue is filed for the main packages, so that they can be resolved, rather than the project forked over and over again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sys-bio/tellurium/issues/475#issuecomment-693215663, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG33OXK26UU6HXEJXPUHMLSGBPJZANCNFSM4RJY3QYQ .

-- Matthias König, PhD. Junior Group Leader LiSyM - Systems Medicine of the Liver Humboldt Universität zu Berlin, Institute of Biology, Institute for Theoretical Biology https://livermetabolism.com konigmatt@googlemail.com https://twitter.com/konigmatt https://github.com/matthiaskoenig Tel: +49 30 2093 98435

fbergmann commented 4 years ago

i mean .. if there is a bug with the interpreter dying then maybe someone could report that to the upstream libraries, as it would be good to know ... as i'm quite sure it would be fixable there. I mean the interpreter dying, rather than giving an exception, would happen, in case there was an exception thrown that wasn't caught. @matthiaskoenig do you have an example that crashes right now with libsbml + sedml / combine? i still have not released new numl binaries ... so don't use that one yet ... its close to being done though.

fbergmann commented 4 years ago

by now i released a new libnuml after the same approach, just in case that caused the issue:

https://pypi.org/project/python-libnuml/

luciansmith commented 4 years ago

Thanks, Frank! We'll also need the python 3.8 version, too (we're currently supporting 3.6, 3.7, and 3.8).

Matthias, I'm still not sure if the te* versions of things actually fix anything (like Python crashing) or not.

fbergmann commented 4 years ago

@luciansmith i see 3.8 numl binaries there, where did you find one missing?

luciansmith commented 4 years ago

Hmm, I swear they weren't there when I first checked yesterday, but they are indeed there today. Maybe it was a cache thing. Anyway, thanks!

matthiaskoenig commented 4 years ago

@Frank I will check this over the weekend. Basically I saw these errorserrors while running my unittests on multiple platforms and python version. There is basically some randomness in how all the pytests are collected on the different runs, so the order of test execution varies between platforms. As a consequence the import order cannot always be determined, because it depends on the order in which tests are run (e.g. one tests imports libsbml another test libsedml, depending on their order the overwriting of the symbols is different. I will check if this works now with the latest library versions.