sys-bio / tellurium

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

Initial values of some parameters cannot be modified via SED-ML #534

Closed jonrkarr closed 3 years ago

jonrkarr commented 3 years ago

Example: https://github.com/biosimulators/Biosimulators_test_suite/tree/dev/examples/sbml-core/Elowitz-Nature-2000-Repressilator

Error:

could not set value for beta, it is defined by an assignment rule, can not be set independently., at int rrllvm::LLVMExecutableModel::setValues(bool (*)(rrllvm::LLVMModelData*, int, double), rrllvm::LLVMExecutableModel::GetNameFuncPtr, size_t, const int*, const double*)
jonrkarr commented 3 years ago

From an XML (SED-ML) perspective, its fine to modify the value of this XML attribute. tellurium raises an exception because its trying to apply modifications to the interpreted model, rather than to the specification (XML) of the model. This illustrates the divergence between tellurium's interpretation of model specifications and that of jLibSEDML (iBioSim, VCell, SBSCL) and BioSimulators (15 more tools) which adhere to the original XML modification spirit of SED-ML.

luciansmith commented 3 years ago

XML or not, tellurium is giving an accurate error message here. If an element has an assignment rule, its initial value is ignored. This is baked into the design of SBML.

jonrkarr commented 3 years ago

The SBML file also has the value. In that case, tellurium simply ignores the value and no error is thrown. In this case, tellurium raises an exception.

luciansmith commented 3 years ago

I'm going to stand by Tellurium's error message here and say that a SED-ML file that assigns a value to an SBML element with an assignment rule is, barring a simultaneous deletion of that assignment rule, an error in simulation design. It is of course legal SED-ML, but Tellurium is still correctly pointing out an actual problem.

I'm more than willing to acknowledge that Tellurium's interpretation of SED-ML can diverge from expectations due to the fact that it involves an interpreted model and not raw XML, but this situation is not one of them, IMO.

luciansmith commented 3 years ago

Given https://github.com/biosimulators/Biosimulators_test_suite/commit/5039bd53d279e12c1a456efb8a4b3f673737cd62 it looks like you're OK with Tellurium's behavior, or at least willing to accommodate it, which I appreciate.

It does look like the new version of the test doesn't actually change anything--the value of PX is changed from 0 to 0. Just in case you wanted a more robust test there...

jonrkarr commented 3 years ago

Presently, the test suite views a subset of SED-ML as essential, and everything else as optional. What is essential vs optional is driven by ensuring that tellurium and VCell pass. This is intended to be temporary. Everything is intended to be moved to essential. Ion has been working to make VCell converge. With this, it will be possible to move more SED-ML features from optional to essential. If Ion is able to share updates to jLibSED-ML with Chris Myers, iBioSim will converge too.

The BioSimulators interface to tellurium now includes two SED-ML interpreters -- the tellurium interpreter and the BioSimulators interpreter. The latter directly uses libRoadRunner. The latter provides stricter compliance with other simulation tools, as well as XPath support, add/remove/replace XML model changes, more consistent KiSAO handling, algorithm substitution, multidimensional reports, data for plots, detailed logs, and other features. The latter is now the default.

Because users just expect things to work and are unlikely to read instructions about the various different implementations of SED-ML, we think we have be stricter about consistency.

hsauro commented 3 years ago

If a piece of sedml says remove a reaction, how do you tell the tool to do that. I can imagine most tools don’t have an api for that or do you just modify the SBML and reload the new model?

luciansmith commented 3 years ago

That sounds good to me--I'm also happy to work on getting Tellurium's internal interpreter to converge with the others.

And yes--jLibsedml and earlier versions of libsedml would execute the model changes for you and give you the new XML.

Nothing (so far as I am aware) is able to execute a model change of the 'remove reaction' type in the middle of a simulation, or even between simulations expecting to preserve model state. Tellurium could do it with the right interface, but implementing our own 'changeXML' is not particularly something that seems worth pursuing at this point.

jonrkarr commented 3 years ago

Our philosophy has been to support SED-ML as completely and consistently as possible. That said, I agree with your sentiment that the semantics of SED-ML changes could be improved.

The XML changes cannot occur inside repeated tasks. As a result, they can be evaluated separately from simulation. Due to the XML semantics, this basically has to be done on the XML specifications of models (we do it on XML element trees in memory).

Motivated in part by Vivarium, we have also introduced another way to programatically execute changes. These changes are processed directly on simulator objects. This is currently restricted to attribute changes, partly because this is the lowest common denominator that works with most tools. However, some tools do have more complete APIs to change models. (For FBC tools these changes diverge from SED-ML/SBML conventions because the XML and simulator representations of models diverge to the point that is hard to build a mapping from one to the other, as we rely on for SBML core).

luciansmith commented 3 years ago

I mean, 'some tools' here includes Tellurium, which can add and remove model elements at will, not to mention changing current state values.

However, putting this into SED-ML seems a bit iffy, at least for L1v4. For now, I would assume that time is better spent trying to be consistent about the SED-ML bits everyone is likely to support, rather than try to figure out how to support a tool-specific feature set in SED-ML.

jonrkarr commented 3 years ago

XML changes are consistently supported across all registered tools, except VCell which is in progress and trllurium's own SED-ML interpreter. This was actually fairly easy to achieve by clearly separating document-level model changes from simulation. This allows changes to be executed by one code, rather than by individual tools.

The second mode of model changing which uses simulator APIs to manipulate simulator memory is also internally consistent across tools, but divergent in a few cases from SED-ML XML-based changes (where simulation tools diverge far from XML). This mode of model change is currently only accessible through our Python APIs, and is not accessible through SED-ML files. This is currently restricted to attribute changes partly for the reason you cite -- simulation tools have very different APIs with different capabilities.

jonrkarr commented 3 years ago

In case it's not clear, our interfaces to COPASI, OpenCOR, PySCES, etc handle SED-ML consistently only because we don't use their own SED-ML interpreters. We never really considered their interpreters because they were so limited or bespoke.