shalinshah1993 / SBSCL

The Systems Biology Simulation Core Library (SBSCL) provides an efficient and exhaustive Java implementation of methods to interpret the content of models encoded in the Systems Biology Markup Language (SBML) and its numerical solution.
https://draeger-lab.github.io/SBSCL/
GNU Lesser General Public License v3.0
0 stars 2 forks source link

Nested RepeatedTasks implementation algorithm #6

Closed niko-rodrigue closed 6 years ago

niko-rodrigue commented 6 years ago

Describe what you want intend to do here in pseudo english. That is much better and easier for other to know if your are in the right track.

shalinshah1993 commented 6 years ago

I worked on implementing repeatedTasks and the basic workflow is like this: Fetch tasks recursively until all repeatedTasks are converted to simple Tasks.

For each repatedTasks (without any nested repeatedTasks), I fetch masterRange and execute each subTask , sorted by order, multiple times, depending on masterRange. Before executing each subTask, I check for resetModel and listOfChange, if any, to be applied.

Can you please go through this simple method getTasksList and verify if the workflow makes sense? The goal of this method is to convert repeatedTasks -> Tasks

As of now, I am unable to completely execute SED-ML sample files from document since simulation type for them is SteadyState or OneStep. I don't think SBSCL supports SteadyState or OneStep simulations since they were introduced in SED-ML L1V2.

https://github.com/shalinshah1993/SBSCL/blob/d3656077437c6bf0efdf224a66b6fd35f0af802e/src/org/simulator/sedml/SedMLSBMLSimulatorExecutor.java#L276

niko-rodrigue commented 6 years ago

The point of describing things in plain text is to not have to go the the code to understand what is being done :-) You have to describe the whole process in more details, not just what you do in the getTastsList method. You cannot take all the tasks recursively because of the implication of reset and model changes. You cannot apply the model changes before the simulation needing them start. And I think when you call sedml.getModelWithId I am not sure you will get the model with you previously applied changes, although I am not absolutely sure of what jlibsedml does but that's something you could try with one or several small junit tests.

shalinshah1993 commented 6 years ago

Okay, what do you think will be the right approach for working with nested repeatedTasks?

niko-rodrigue commented 6 years ago

I would have thought that @draeger or @matthiaskoenig would dig in a bit on this issue, may be they will do it later. I will let you thing a bit more why what you want to do is wrong and how it can be done because that's the whole point of GSOC. I am trying to make you understand a bit more how the sed-ml repeated task works.

Hint: check carefully section 2.4.6.4 of the sed-ml L1V2 specifications.setValue inherit from computeChange so it can have a listOfVariables, there is only one case where we take the values from the initial state of the model, otherwise you have to use the values obtain at the end of the previous task/simulation (whether it is a repeatedTask or a normal task).

shalinshah1993 commented 6 years ago

Based on the understanding, I have created a List which stores modelState. I clear it only if resetModel is true otherwise I grab a fresh model and always update its value to previous state. See if this makes sense. https://github.com/shalinshah1993/SBSCL/commit/e737682a04bad53b407764fbe6ca1aa7123c4bfb

niko-rodrigue commented 6 years ago

On line 291 you need to add a else case for repeatedTask. When we speak about the state of the model, it correspond to the full simulation results, you need to use the simulation results to set the initial values of all the model elements for the next simulation. Once you have done that, you can apply the listOfChanges if there are some.

As I said in slack, you cannot flatten the repeatedTask, if you encounter one, you have to run it to the end, including all the repetitions (not just one repetition). Then you have the final simulation results that can be used for the next subTask or next repetition of the parent repeatedTask. Am I making sense ?

shalinshah1993 commented 6 years ago

Yes, you totally do! So, right now I get (flattened) taskList first and then execute each Task. When I flatten the list, I make sure each subTask gets the right variable assignments as per listOfChanges and resetModel. That means I have to change run() workflow. I will have to brainstorm how that can work especially for nested repeatedTasks.