sbmlteam / libsbml

LibSBML is a native library for reading, writing and manipulating files and data streams containing the Systems Biology Markup Language (SBML). It offers language bindings for C, C++, C#, Java, JavaScript, MATLAB, Perl, PHP, Python, R and Ruby.
https://sbml.org/software/libsbml
Other
41 stars 28 forks source link

Improve the efficiency of the AssignmentCycles::determineAllDependencies() function #374

Closed eltix closed 3 months ago

eltix commented 6 months ago

Hi all,

The AssignmentCycles::determineAllDependencies() function does not behave well with models having many parameters and rules and transitive dependencies between model constructs. Unfortunately, I cannot share any of the actual models we have had issues with for they are confidential. However, I have crafted a couple of examples (see attachments) which exhibit somewhat similar structures.

For our actual models, this function takes a prohibitive time (> 1hour) to terminate even though the ODE system solving part takes less than a minute. As a consequence, we have temporarily bypassed the consistency checks altogether in our pipelines.

I would gladly get involved myself in the search for a suitable solution but my C++ skills are alas quite limited. Thank you all for maintaining this library!

Attachments

determineAllDependenciesInputs.zip

Contains:

  1. the "small" Model150.xml example
  2. the "medium" Model180.xml example
  3. the "large" Model300.xml example
  4. A runConsistencyChecks.cpp file to load a document and run the consistency checks
fbergmann commented 5 months ago

Without a model it will be really hard for us to determine whether we managed to tackle the performance issue. Could you perhaps run the validation once (overnight) for your 150 /180 model with valgrind:

valgrind --tool=callgrind ./<executable that validates your model>

the validate c++ example would work fine for that.

thank you

eltix commented 5 months ago

Thank you for your answer @fbergmann I don't really understand what you mean by "Without a model"? All three models are in the zip bundle I attached to the issue description.

fbergmann commented 5 months ago

indeed, nevermind. running cachegrind now to see where this is coming from

fbergmann commented 5 months ago

maybe you can try the code from the branch, with that the model300 completes in a couple of seconds for me

eltix commented 5 months ago

Thank you @fbergmann, I confirm your fix drastically improves the performance. For our problematic model, we went from close to infinite time to 2min30s which is already a great improvement.

fbergmann commented 5 months ago

You might want to think about encoding your model in SBML L2V1, there the restriction for assignment cycles does not hold, so the test would be skipped altogether. Since your model seems to use no features from L3V2 it might be something worth considering.