sys-bio / tellurium

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

[SEDML] xpath expression resolving with models #114

Open matthiaskoenig opened 8 years ago

matthiaskoenig commented 8 years ago

Currently the SEDML xpath expressions are resolved via heuristic from xpath string without using the xml of the model. This should be changed to use xpath from xml library with model xml.

This current solution results in a handful of problems.

r['S1'] = 1.0 # parameter
r['init(S1)'] = 1.0  # species amount
r['init([S1])'] = 1.0 # species concentration
luciansmith commented 8 years ago

I wrote those XPaths this way because I knew libsbml could resolve them easily:

target = model.getElementBySId(id) type = target.getType()

'type' will then contain SBML_SPECIES, SBML_PARAMETER, or whatever.

matthiaskoenig commented 8 years ago

Okay, this helps.

The given xpath expression is the only reason I have to parse the SBML model for creating executable python code. Without parsing the SBML/XML it is not possible to find out what is meant by the xpath. But at latest for applying the XMLChanges I need the representation of SBML/XML in memory.

@luciansmith You mentioned that you opened some SBML issue for

getElement(s)ByXPath(xpath)   # gets Element(s) from xpath
addXML(xpath, xml)               # adds xml to the element specified by xpath
changeXML(xpath, xml)               # changes xml 
removeXML(xpath)                # remove xml at xpath

Could you comment on this? Is this filed? Otherwise I would make a request on sbml-develop

luciansmith commented 8 years ago

I did indeed file this as a feature request in libsbml's pivotaltracker:

https://www.pivotaltracker.com/n/projects/248655/stories/114704823

It doesn't look like it's going to be implemented any time soon, though. Personally, I think SED-ML's choice to use XPath in the first place was a mistake, or at least that there should be alternative methods for pointing at things in known specifications. I guess this is beside the point, though.

In the meantime, I just changed phrasedml's output to be the easier-to-parse XPaths in most cases, though it will default to the old style for edge cases like for IDs that are not the 'standard' elements (like might show up in packages). I also proposed a way to use Antimony for your other XML replacement scheme, which might at least stave things off a while.

The only robust method I can think of for this at the moment is:

I don't really know how to do this on the phrasedml side at all. I do have a proposal for VERY GENERIC additions and replacements, but nothing for changes, and the proposals I do have don't even cover your two use cases.

The only thing I can think of (which might work) is to code up your alternate SBML in a different model entirely, and then say something like:

model1 = model "main.xml" model2 = model "bits_and_pieces.xml" model3 = model model1 with S1 = model2.S1 model4 = model model1 with p1 = model2.p1 model5 = model model1 with add model2.p1.rateRule model6 = model model1 with remove S3

matthiaskoenig commented 8 years ago

Yes, xpath is a bit of a problem. Everybody will implement them differently and many tools will only parse a subset of the possible expressions. That is why I will use general xml library to let it solve the problem (so tellurium can understand all the possible xpath expressions others could write). So from the tellurium side I do not care much about the xpath encoding (we just accept everything which are valid xpaths and resolve to one object in the sbml), but for round tripping with other tools the more specific xpath are much better.

I like the idea of bits_and_pieces. This is exactly what I would do either with libsbml or antimony and than refer to them in the sedml. I do not want to have xml strings anywhere, but something which resembles parts of a model. Otherwise everything will break.

matthiaskoenig commented 8 years ago

Hi Lucian, I cannot comment on the tracker https://www.pivotaltracker.com/n/projects/248655/stories/114704823

I feel very uncomfortable working with the raw XML (I am not a computer scientist, I am a computional biologist). The core idea of libsbml and libsedml is to provide an abstraction layer around the XML.

The xpath expressions & the XMLChanges are the only cases where I now have to really manipulate/work with the XML, i.e. xml parser, xpath parser, changing the dom. All things which are highly error prone and should be abstracted away by the respective libraries. Not implementing a proper support by libsbml or libsedml for this will cause much more problems in the long run because none of the xpath and xmlChanges will be standardized. So you suddenly have a standard which allows that everybody can do what they want (which is by definition not a standard any more). This will break compatibility on so many levels. This will be the main reason why SEDML will not be reproducible between different tools.

matthiaskoenig commented 7 years ago

This must be fixed, some combine archives make use of this See for instance

<changeAttribute target="/sbml:sbml/sbml:model/sbml:listOfReactions/sbml:reaction[@id=&apos;reaction_9&apos;]/sbml:kineticLaw/sbml:listOfLocalParameters/sbml:localParameter[@id=&apos;KmP2&apos;]/@value" newValue="73.6319"/>

in https://jjj.bio.vu.nl/models/experiments/levering2012_fig5-user/ which breaks with

                Traceback (most recent call last):
  File "/home/mkoenig/envs/tellurium-web/lib/python3.5/site-packages/celery/app/trace.py", line 374, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/home/mkoenig/envs/tellurium-web/lib/python3.5/site-packages/celery/app/trace.py", line 629, in __protected_call__
    return self.run(*args, **kwargs)
  File "/home/mkoenig/git/tellurium-web/teweb/combine/tasks.py", line 63, in execute_omex
    dgs_all = tellurium.sedml.tesedml.executeOMEX(omex_path, workingDir=tmp_dir)
  File "/home/mkoenig/git/tellurium/tellurium/sedml/tesedml.py", line 191, in executeOMEX
    sedml_dgs = factory.executePython()
  File "/home/mkoenig/git/tellurium/tellurium/sedml/tesedml.py", line 409, in executePython
    exec(compile(execStr, filename, 'exec'), symbols)
  File "/tmp/te-generated-sedml.py", line 82, in 
    model3_levering2['reaction_9'] = 73.6319
  File "/home/mkoenig/envs/tellurium-web/lib/python3.5/site-packages/roadrunner/roadrunner.py", line 3353, in __setitem__
    return _roadrunner.RoadRunner___setitem__(self, id, value)
RuntimeError: The sbml id 'reaction_9' is for a reaction, reaction rates can not be set externally
matthiaskoenig commented 7 years ago

Test cases added for this issue

    # complex xpath expressions: FIXME: https://github.com/matthiaskoenig/tellurium-web/issues/52, https://github.com/sys-bio/tellurium/issues/114
    'jws/omex/levering2012_fig5-user.sedx',
    'jws/omex/levering2012_fig2-user.sedx',