draeger-lab / 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
20 stars 13 forks source link

Add support for rateOf csymbol #43

Closed hemilpanchiwala closed 4 years ago

hemilpanchiwala commented 4 years ago

A new rateOf csymbol has been introduced to the SBML Level 3 Version 2. Support for rateOf needs to be added to SBSCL which can be done by creating a function calculating a rate of any particular variable that needs to be added in the ASTNodeInterpreter class of SBSCL. Currently the SBML L3V2 models containing rateOf symbol outputs NaN values as results.

hemilpanchiwala commented 4 years ago

@draeger @matthiaskoenig, can you once specify how will the rateOf be calculated for the Compartment or Parameter Node?

hemilpanchiwala commented 4 years ago

@draeger @matthiaskoenig Now, the rateOf method has been added to the SBSCL simulator and now many tests with rateOf tests are passing now. (Link to the rateOf method)

However, there is a problem for time t = 0 in some test cases, as the rateOf is called before the derivatives are computed which leads to wrong results and thus failed tests. This occurs for the test cases 1256, 1257, and some others.

Screenshot of the results from the SBML Test Runner. (Difference in results is at t = 0 only) Screenshot from 2020-06-15 15-51-31

matthiaskoenig commented 4 years ago

Here some hints for the implmentation:

hemilpanchiwala commented 4 years ago

@matthiaskoenig, I have implemented this by passing the instance of SBMLinterpreter in the rateOf method and getting the derivatives calculated by computeDerivatives method in SBMLinterpreter. The only problem I get is that at t = 0, the rateOf is calculated even before the derivatives are computed so gets the derivative field null.

hemilpanchiwala commented 4 years ago

Here is the implementation: https://github.com/hemilpanchiwala/SBSCL/blob/b73b2931cb7b4d5318ff19076dad1893ee076213/src/main/java/org/simulator/sbml/astnode/ASTNodeInterpreter.java#L1095

matthiaskoenig commented 4 years ago

I nowhere see the handling of the constant symbols or the combined derivatives of concentration species?

Also put things outside of loops if you can:

for (int i = 0; i < sbmlInterpreter.getRateRulesRoots().size(); i++) {
      if (sbmlInterpreter.getRateRulesRoots().get(i).getVariable().equals(sBase.getId())) {
        return sbmlInterpreter.getRateRulesRoots().get(i).getNodeObject().compileDouble(time, 0d);
      }
    }

Here you do in every loop iteration four possibly expensive unnecessary function calls. No sure this is optimized away, but much better to do things outside of loops and remove duplicate calls:

rateRulesRoots = sbmlInterpreter.getRateRulesRoots()
for (int i = 0; i < rateRulesRoots.size(); i++) {
     rrRoot = rateRulesRoots.get(i);
      if (rrRoot.getVariable().equals(sBase.getId())) {
        return rrRoot.getNodeObject().compileDouble(time, 0d);
      }
    }
matthiaskoenig commented 4 years ago

So you have to figure out how to evaluate the derivatives before the rateOfs if rateOfs are in the model.

hemilpanchiwala commented 4 years ago

I nowhere see the handling of the constant symbols or the combined derivatives of concentration species?

@matthiaskoenig, I will add handling for the constant symbols but the species with concentration or amount units are being handled in the changeRate array (calculated from computeDerivatives) itself in SBMLinterpreter.

hemilpanchiwala commented 4 years ago

I nowhere see the handling of the constant symbols.

Link to the rateOf function: https://github.com/hemilpanchiwala/SBSCL/blob/testrunner/src/main/java/org/simulator/sbml/astnode/ASTNodeInterpreter.java#L1095 @matthiaskoenig, Does this seem okay now or should I make all the if statements for returning 0d in just one if statement?

Also, the duplicate code in for loop has been moved out of the loop.

matthiaskoenig commented 4 years ago

Just calculate once the constant base in a hash map and look them up instead of doing all this tests every integration step.

On Tue, Jun 16, 2020, 20:52 Hemil Panchiwala notifications@github.com wrote:

I nowhere see the handling of the constant symbols.

Link to the rateOf function: https://github.com/hemilpanchiwala/SBSCL/blob/testrunner/src/main/java/org/simulator/sbml/astnode/ASTNodeInterpreter.java#L1095 @matthiaskoenig https://github.com/matthiaskoenig, Does this seem okay now or should I make all the if statements for returning 0d in just one if statement?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/draeger-lab/SBSCL/issues/43#issuecomment-644949613, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG33OWJ6GEZ6BVZ6YBGXEDRW65OXANCNFSM4N2IXCIQ .

hemilpanchiwala commented 4 years ago

@matthiaskoenig, added a constant hashmap in SBMLinterpreter and updated the rateOf method in the linked commit above. Does it seem fine now? Updated rateOf method [Link].

hemilpanchiwala commented 4 years ago

I also find one test case of rateOf which fails but I don't get any clue for it. Test case 1456.