sys-bio / roadrunner

libRoadRunner: A high-performance SBML simulator
http://libroadrunner.org/
Other
39 stars 25 forks source link

Simulation with negative start time does not work #411

Closed matthiaskoenig closed 3 years ago

matthiaskoenig commented 7 years ago

It is valid to run simulations from a negative start time (start is only how the value is called which corresponds to the the start of the simulation). I.e. if I want a plot from -20 to 0 this is completely valid, but does not work in roadunner:

r.simulate(start=-20, end=0, steps=10)

~/envs/tellurium/lib/python3.5/site-packages/roadrunner/roadrunner.py in simulate(self, start, end, points, selections, steps)
   3577             o.steps = steps
   3578 
-> 3579         result = self._simulate(o)
   3580 
   3581         o.steps = originalSteps

~/envs/tellurium/lib/python3.5/site-packages/roadrunner/roadrunner.py in _simulate(self, opt)
   3330 
   3331     def _simulate(self, opt):
-> 3332         return _roadrunner.RoadRunner__simulate(self, opt)
   3333 
   3334     def getValue(self, *args):

RuntimeError: duration, startTime and steps must be non-negative

The only requirement is that start <= end, but not non-negativity.

This is needed for SED-ML simulations which use negative start times in combination with offsets.

luciansmith commented 7 years ago

Are there examples that actually use this? I tend to think that this might be a problem with the SED-ML spec.

-Lucian

hsauro commented 7 years ago

Is there a use case for having negative start times?

Herbert

On Mon, Sep 18, 2017 at 8:09 AM Matthias König notifications@github.com wrote:

It is valid to run simulations from a negative start time (start is only how the value is called which corresponds to the the start of the simulation). I.e. if I want a plot from -20 to 0 this is completely valid, but does not work in roadunner:

r.simulate(start=-20, end=0, steps=10)

~/envs/tellurium/lib/python3.5/site-packages/roadrunner/roadrunner.py in simulate(self, start, end, points, selections, steps) 3577 o.steps = steps 3578 -> 3579 result = self._simulate(o) 3580 3581 o.steps = originalSteps

~/envs/tellurium/lib/python3.5/site-packages/roadrunner/roadrunner.py in _simulate(self, opt) 3330 3331 def _simulate(self, opt): -> 3332 return _roadrunner.RoadRunner__simulate(self, opt) 3333 3334 def getValue(self, *args):

RuntimeError: duration, startTime and steps must be non-negative

The only requirement is that start <= end, but not non-negativity.

This is needed for SED-ML simulations which use negative start times in combination with offsets.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sys-bio/roadrunner/issues/411, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAZDq0Uk5glgCTpYf3V9jcszTMzU3Foks5sjodUgaJpZM4PbDuh .

matthiaskoenig commented 7 years ago

Yes, use case is here: https://jjj.bio.vu.nl/models/experiments/martins2016_fig4b/

<listOfSimulations>
    <uniformTimeCourse id="sim0_model0_sigcpsba" initialTime="-20" outputStartTime="0" outputEndTime="120" numberOfPoints="1000">
      <algorithm kisaoID="KISAO:0000019"/>
    </uniformTimeCourse>
  </listOfSimulations>

There should be a initial integration duration of 20 which should not be depicted in the figure (the initial phase to oscillations), but the start time of the plot should be at 0.

This also often happens in experimental data where the pre-intervention phase is negative (for instance -100 to 0, than intervention is given from 0 to 10. The simulation would start at negative times and the trigger for an intervention event would be triggered at 0.

matthiaskoenig commented 7 years ago

And specification looks good. It does not exclude negative start times.

2.4.3.3.1 initialTime The attribute initialTime of type double represents the time from which to start the simulation. Usually this will be 0

2.4.3.3.2 outputStartTime Sometimes a researcher is not interested in simulation results at the start of the simulation (i.e. the initial time). To accommodate this in SED-ML the uniformTimeCourse class uses the attribute outputStartTime of type double. To be valid the outputStartTime cannot be before initialTime. For an example, see Listing 2.44.

luciansmith commented 7 years ago

This is really problematic, because 'time' has a specific meaning in an SBML model, and by definition in SBML, starts at 0 at the start of the simulation. If the model uses csymbol 'time' in it, it will mean 'the variable that starts at 0 at the beginning of the simulation'. CellML could handle this, since in that language, 'time' is a variable just like any other, and if you want to set its initial value to -20, you just set it. But there's no way to set the csymbol 'time' in SBML.

You could fake it well enough if you ran the simulation as if -20 was 0, and then shifted the results back 20. You could do this entirely on the Tellurium side: it would take the input 'run from -20 to 50', tell roadrunner 'run from 0 to 70', then subtract 20 from the 'time' column of data. This would work for all models that don't use csymbol 'time' in them. But it would fail (or at least behave differently from CellML) for any models that did. Maybe have Tellurium issue a warning for all SED-ML that requests a negative start time if the model contains csymbol 'time'?

matthiaskoenig commented 7 years ago

Yes, I understood the -20 as a simple time shift. I.e. start and end time of the simulation only define the duration for the simulator, independent of these values the model is run from 0 to duration=(end-start). The resulting time vector is than shifted to start at start (for instance at -20). I assume the same is happening for setting a positive start value for instance 20 right now in roadrunner, so I don't understand why the same is not possible to do for -20. Just shift the time vector to -20 instead of to 20.

In summary r.simulate(start=0, end=20, steps=100) r.simulate(start=20, end=40, steps=100) r.simulate(start=-20, end=0, steps=100) Should give the exact same numerical results, only the time vector is shifted to the start time.

@lucian "t starts at 0 at the beginning of the simulation" is the same issue for +20 shifts which are currently supported by roadrunner.

On Mon, Sep 18, 2017 at 7:04 PM, Lucian Smith notifications@github.com wrote:

This is really problematic, because 'time' has a specific meaning in an SBML model, and by definition in SBML, starts at 0 at the start of the simulation. If the model uses csymbol 'time' in it, it will mean 'the variable that starts at 0 at the beginning of the simulation'. CellML could handle this, since in that language, 'time' is a variable just like any other, and if you want to set its initial value to -20, you just set it. But there's no way to set the csymbol 'time' in SBML.

You could fake it well enough if you ran the simulation as if -20 was 0, and then shifted the results back 20. You could do this entirely on the Tellurium side: it would take the input 'run from -20 to 50', tell roadrunner 'run from 0 to 70', then subtract 20 from the 'time' column of data. This would work for all models that don't use csymbol 'time' in them. But it would fail (or at least behave differently from CellML) for any models that did. Maybe have Tellurium issue a warning for all SED-ML that requests a negative start time if the model contains csymbol 'time'?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sys-bio/roadrunner/issues/411#issuecomment-330288098, or mute the thread https://github.com/notifications/unsubscribe-auth/AA29ulnHXUiTi5TlpJ_9JngIbtiQBVYuks5sjqKpgaJpZM4PbDuh .

-- Dr. Matthias König Junior Group Leader LiSyM - Systems Medicine of the Liver Humboldt-University Berlin, Institute of Biology, Institute for Theoretical Biology https://www.livermetabolism.com konigmatt@googlemail.com Tel: +49 30 20938450 Tel: +49 176 81168480

kirichoi commented 7 years ago

If that's the case, the fix should be pretty simple. We can simply store the initial delta T, simulate from 0 nonetheless, and add delta T at the end to time vector.

luciansmith commented 7 years ago

I still worry that both the -20 and the +20 solutions are not what was intended by SED-ML, but I suppose as early adopters of the spec, we have a say in what the spec means--if we're setting up the SED-ML test suite by default, everyone else has to do it our way ;-)

luciansmith commented 7 years ago

As a followup: discussed this with the editors, and this solution is indeed the correct one: whatever state the model is in is assumed to be the 'initial state', and if 'time' is supposed to be -20 or +20 or whatever, it must itself deal with the fact that the csymbol 'time' in SBML means 'time from the simulation initial state', and add a delta itself. (i.e. it's a modeling issue, not a software issue.)

kirichoi commented 7 years ago

Good to here that. I will implement this solution asap.

matthiaskoenig commented 7 years ago

thanks

0u812 commented 7 years ago

As Lucian pointed out, this causes problems with events etc. (#425).

luciansmith commented 7 years ago

I wonder if we could solve this by splitting 'time' into two columns: something like 'internal time' and 'simulation time'. You would treat 'internal time' by leaving it as-is, and it would start with zero and end wherever. Then 'simulation time' would be offset by the required SED-ML shift. (The shift is not supposed to affect the internals of the model at all, including events, etc. But it will look weird if you know your event trigger is 'time>5' and it fires at time="-3".

kirichoi commented 7 years ago

This is indeed problematic when dealing with events. Not only that, it's already a problem when starting simulation from time points other than 0. E.g. when you run:

r = te.loada("""
model mymodel()
  var species X = 10
  at time>=5: X = 5
end
""")

result = r.simulate(3, 10)
r.plot()

image I am not sure whether it's feasible, but one way to solve this issue might be adding delta T = 0 - starttime to all time values specified in events and apply the previously proposed solution.

hsauro commented 7 years ago

Should the 5 in time>=5 be relative to the current time or should it be the absolute time. If it were the absolute time it would work.

What does the SBML spec say?

If time is meant to be absolute then I guess we're interpreting it wrongly?

Herbert

On Wed, Oct 11, 2017 at 5:09 PM, Kiri Choi notifications@github.com wrote:

This is indeed problematic when dealing with events. Not only that, it's already a problem when starting simulation from time points other than 0. E.g. when you run:

r = te.loada(""" model mymodel() var species X = 10 at time>=5: X = 5 end """)

result = r.simulate(3, 10) r.plot()

[image: image] https://user-images.githubusercontent.com/7672320/31472790-a1d09c72-aea5-11e7-9342-d438ce208be0.png I am not sure whether it's feasible, but one way to solve this issue might be adding delta T = 0 - starttime to all time values specified in events and apply the previously proposed solution.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sys-bio/roadrunner/issues/411#issuecomment-335983154, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAZDu8_OBLpuaqSdyBf-1HQ5_GBPqj-ks5srVibgaJpZM4PbDuh .

kirichoi commented 7 years ago

Reading SBML spec, it seems like time is actually treated as an absolute value. That means not only our proposed solution is wrong, but also the way we handle simulation starting at t >0 is wrong. Simulation always starts at t = 0, event always triggers at specified time point (t > 0) If one start simulation at t > 0, simulation still starts from t = 0 but the output must be truncated. If one start simulation at t < 0, neither reactions nor events will start. Only specific rules (AssignmentRule, AlgebraicRule, etc.) may take effect.

luciansmith commented 7 years ago

So, this is a confusing situation and I've tried to update the latest SED-ML spec to be more clear, but the upshot is:

You are supposed to run your model as-is, without changing the internal 'time' at all. The assumption is that if a SED-ML file tells you to that the 'initialTime' is -20, that means that the model has already been scaled so that when the internal time is zero, the reported time should be -20.

The upshot for tellurium is that SED-ML's initialTime is a post-processing step: it should be applied to roadrunner's output, not any input. You truncate your output if (and only if) the 'outputStartTime' is different from the 'initialTime'.

luciansmith commented 3 years ago

OK, coming back to this after four years...

From my perspective now, I think that the SED-ML decision was the wrong one: when simulating an SBML model, time starts at 0. It always has, and there's no way around it, since it's an implied variable. The call:

simulate(5, 10, 50)

doesn't request that simulation start at 0, but that output collection begin at 5 and go to 10. As such, a negative number as the 'start' argument cannot make sense in a roadrunner context.

If SED-ML wants to declare that non-zero 'initialTime' attributes mean 'the pre-initialized state of the model as loaded' that would be a tellurium issue with interpreting SED-ML, but not anything I would want to implement as part of the 'simulate' function.

For Matthias: if you want to declare that time zero is actually time -20, I would say that this is a post-processing step you could perform the data that roadrunner outputs. But since 'time' is an internal variable in an SBML model, there's no way to make roadrunner do what you want.

matthiaskoenig commented 3 years ago

Okay, this makes sense. But this should then be a validation rule/warning on SED-ML. I.e. if the model is an SBML model and simulation times are negative this is invalid.

luciansmith commented 3 years ago

I looked at this yet again due to Herbert saying something, and I was wrong--our simulation start time isn't a data collecction start, but an actual 'we assume the model is set to this time' start. This in turn completely changed my perspective on what 't0' meant in an SBML model, and it made data collection with t0 < 0 actually much easier!

It turned out to be difficult to get pre-time==0 events to fire correctly, but I have them working in general--still need to test more, but it's looking good: https://github.com/sys-bio/roadrunner/pull/854

luciansmith commented 3 years ago

This is now done! The only thing that doesn't work is https://github.com/sys-bio/roadrunner/issues/857 which isn't a negative-start-time specific problem. So I'm claiming this is done! It'll be in the next release.

luciansmith commented 3 years ago

Also, if you have opinions on this matter, other perspectives would be helpful at https://github.com/sbmlteam/sbml-specifications/issues/2