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

infix math from libsedml uses '^' instead of '**' in python, validation fails #118

Closed
jcschaff opened this issue Oct 17, 2022 · 4 comments · Fixed by #119
Closed

infix math from libsedml uses '^' instead of '**' in python, validation fails #118

jcschaff opened this issue Oct 17, 2022 · 4 comments · Fixed by #119
Assignees
Labels
bug Something isn't working SED-ML

Comments

@jcschaff
Copy link
Member

jcschaff commented Oct 17, 2022

if the MathML for a computeChange contains a 'pow', the infix notation from returned by libsedml.formulaToL3String() represents the pow operator as '^' and python represents it as '**'. During validation of a computed change, this infix representation (stored in change.math) is evaluated and fails if the operands are not compatible with bitwise XOR (e.g. if either operand is a float)

The suggested fix in Biosimulators_utils/sedml/math.py is to replace '^' with '**' as follows:

def compile_math(math):
    """ Compile a mathematical expression

    Args:
        math (:obj:`str`): mathematical expression

    Returns:
        :obj:`_ast.Expression`: compiled expression
    """
    if isinstance(math, str):
        math = (
            math
            .replace('&&', 'and')
            .replace('||', 'or')
            .replace('^', '**')
        )

    math_node = evalidate.evalidate(math,
                                    addnodes=VALID_MATH_EXPRESSION_NODES,
                                    funcs=MATHEMATICAL_FUNCTIONS.keys())
    compiled_math = compile(math_node, '<math>', 'eval')
    return compiled_math

@luciansmith
Copy link
Contributor

There's other issues with the mathml-to-infix converter, most obviously the inclusion of units--you can get stuff like "1 mL * S1" and the like. And you'll also have issues with true, false, avogadro, other built-in functions... I'll make a list and file a new bug report.

@jcschaff
Copy link
Member Author

@luciansmith it seems that we can avoid some of these issues if we work directly with the expression ASTNodes, or ask libsbml/libsedml to validate and/or evaluate the expression for us.

@jcschaff
Copy link
Member Author

@jonrkarr please weigh in on this issue. This power operator is blocking VCell/Biosimulators interoperability.

@jcschaff
Copy link
Member Author

I didn't read Lucian's post carefully enough - he suggested to file a new Issue for expanded expression compatibility support (e.g. units). I will merge the associated approved PR (by Eran) and close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SED-ML
Development

Successfully merging a pull request may close this issue.

2 participants