Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support StoichiometryMath #258

Closed
sebapersson opened this issue Oct 20, 2023 · 8 comments
Closed

Support StoichiometryMath #258

sebapersson opened this issue Oct 20, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@sebapersson
Copy link
Contributor

Target functionality

In SBML level 2 StoichiometryMath is allowed (should be noted it is not supported for SBML level 3). The code I would like to work is;

using SBML
using Downloads
path = Downloads.download("https://raw.githubusercontent.com/sbmlteam/sbml-test-suite/release/cases/semantic/00068/00068-sbml-l2v5.xml", joinpath(@__DIR__, "model.xml"))
model = readSBML(path)
r1 = model.reactions["reaction1"]
r1.products[1].stoichiometry

Desired output

Here as this reaction has a StoichiometryMath element the return would be (in the form of MathApply):

2*p1

Optional: Suggestions for implementation

The relevant documentation can be found here https://sbml.org/software/libsbml/5.18.0/docs/formatted/c-api/class_stoichiometry_math__t.html. I am not entirely sure how to add it, but I figure that the best approach would be to allow for SBML.SpeciesReference.stoichiometry to be a more advanced math expression in case stoichiometryMath is present in the model.

This is maybe most relevant feature as stoichiometryMath only is supported for level2. However, as our PEtab.jl SBML importer currently supports majority of features for ODE models (except primarily events with delay, events with priority, and species conversion factor see #257), for completness it would be awesome to have this features!

@exaexa
Copy link
Collaborator

exaexa commented Oct 20, 2023

Hi! Just a quick question, I see that SBML actually has 2 different datafields for this:
https://github.com/sbmlteam/libsbml/blob/development/src/sbml/SpeciesReference.h#L1265-L1267

In order not to complicate&break our existing .stoichiometry field with extra types, would it be OK for you to have this separately in .stoichiometry_math field? Normally I'd try to avoid making extra fields because we should be able to handle cleanly with types, but given libsbml actually has these as very separate data pieces with not-a-very-clarified relation (see setStoichiometryMath in the .cpp file for the above), we're probably much better off with having both.

Note to self: Looking at the libsbml sources, we should probably also have the denominator field handy somewhere...

@exaexa exaexa added the enhancement New feature or request label Oct 20, 2023
@paulflang
Copy link
Collaborator

(Not sure if relevant, but in the early days I think we decided to only focus on supporting SBML L3V2 and use set_level_and_versions for everything else. Would that not work here?)

@exaexa
Copy link
Collaborator

exaexa commented Oct 24, 2023

@paulflang yeah good point. Kinda wondering what would the conversion do in this case though, is there any place where the math would get offloaded?

@sebapersson
Copy link
Contributor Author

A check from my part, would set_level_and_versions be able to transform level 2 SBML model with StoichiometryMath to a level 3 model (with the StoichiometryMath correctly handled?)

@paulflang
Copy link
Collaborator

paulflang commented Oct 24, 2023

Maybe it will be offloaded in an assignmentRule? But I have not tried.

@exaexa
Copy link
Collaborator

exaexa commented Oct 24, 2023

looks from here https://github.com/sbmlteam/libsbml/blob/1e15d62146d96686f9a321a305b292aa8195cc49/src/sbml/SBMLConvert.cpp#L1120-L1149 like it actually converts stuff to assignmentRules.

@sebapersson can you check if this would be OK for you? it should convert the rules as follows:

  • there is going to be a new parameter identified by SpeciesReference field id
  • the SpeciesReference is gonna be marked as not constant
  • the math will be in an assignment rule identified by the id form S.R.

If there is any problem with the conversion, we could actually implement this quite easily -- there's support for math parsing already, and you can add it to the SpeciesReference parsing as a new field with a bit of copypaste, just as with the 2 fields in the other issue....

@sebapersson
Copy link
Contributor Author

Thanks a lot, this works well for the case above!

I still have to go through the SBML test-suite to see that no problem pops up with the conversion, and if that happens I will reopen the issue.

@exaexa
Copy link
Collaborator

exaexa commented Oct 25, 2023

OK, thanks for the info! Please feel free to reopen with a testsuite ID in case you find any problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants