-
Notifications
You must be signed in to change notification settings - Fork 2
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
AlgorithmParameter 'value' strings must be non-empty. #166
Comments
Added this sentence to the end of the 'value' paragraph: "The string must be non-empty; to explicitly state that a value is not set, this should be |
I created KISAO_0000629 for this. I hope this can be addressed in a way that's clearer in L2 as part of better support for data types. |
How would you recommend using KiSAO 629 as a 'value'? Do you envision:
or
or some other way? |
The former looks like other uses of KiSAO in SED-ML. We're also using this scheme to indicate nested algorithm choices, such as for nested choices of CVODE supported by OpenCOR and nested choices for optimization methods from COBRApy and CBMPy. In BioSimulators, we describe that the expected data type of these algorithm parameters for OpenCOR, etc. is a |
I would prefer the formulation of: "The string must be non-empty; to explicitly state that a value is not set, this should be to what we have currently in the draft spec |
(Sorry I missed this before submission...)
Argh; missed this before submission. Changed; maybe we'll get it in the final version. |
cf fbergmann/libSEDML#88: empty 'value' strings are treated by libsedml as unset, as has been the common practice in libsbml and other systems biology libraries for yonks.
However, this came as a surprise to @jonrkarr , who expected to be able to 'explicitly not set' an algorithm parameter by doing something like:
It would be technically possible to update libsedml to handle this, but then we'd have to re-train developers who expect that if 'getX' returns "", that means that X hasn't been set, too. In general, it seems easier to just say in the spec that if you want to explicitly set something to be 'unset', you have to use something other than an empty string.
The text was updated successfully, but these errors were encountered: