operator precedence not always respected in arithmetic structures from Abstract model, missing brackets

[lionel42]

Hello ! I came across something quite weird while playing around with the abstract model.

As I understand the ways ArithmeticStructure (ARS) work is that if an ARS is an argument of another ARS, the nested ARS will have its operations performed before the main ARS. For example:

ARS([*], [
   1, ARS([+, +], [
          2,3,4
]

Should be understood as 1* (2+3+4)

And it seems to be the default behavior for building Abstract model as far as I understood.

If we would invert the and the + operators in the previous example we could remove the brackets as they become unnecessary, and it seems that pysd removes them automatically during building. For example ` 1+ (234)is the same as 1+ 23*4`

Now I found out that in some cases pysd is removing brackets when it should not

ARS([*], [
   1, ARS([+, ^], [
          2,3,4
]

This should result in 1 * (2 + 3**4) but what pysd gives is 1 * 2 + 3**4 which leads to a different equation !

This can be solved for the moment by having ARS containing only operators with the same precedence, it could be an easy solution to raise an error when operators with different precedence are put in the same ARS, but it would be limiting for future developments.

Otherwise it could be nice that the builder handles this edge cases. (I was personally a bit lost when I had a look to the code where this thing happens)

I put a test code for this: (I did not manage to build only the AST without having to build a full model :'( )

from pathlib import Path
from pysd.translators.structures import abstract_expressions, abstract_model
from pysd.builders.python.python_model_builder import ModelBuilder

path = Path("test.py")
model = abstract_model.AbstractSection(
    name="Test",
    path=path,
    type="main",
    params=[],
    returns=[],
    subscripts=(),
    split=False,
    views_dict=None,
    constraints=(),
    test_inputs=(),
    elements=(
        abstract_model.AbstractElement(
            "test",
            components=[
                abstract_model.AbstractComponent(
                    ([], []),
                    ast=abstract_expressions.ArithmeticStructure(
                        ["*"],
                        [
                            1.0,
                            abstract_expressions.ArithmeticStructure(
                                ["+", '^'],
                                [
                                    2,
                                    3,
                                    4
                                ],
                            ),
                        ],
                    ),
                )
            ],
        ),
    ),
)
model = abstract_model.AbstractModel(original_path=path, sections=(model,))

original_model = ModelBuilder(model)
f_name = original_model.build_model()

[enekomartinmartinez]

Hi @lionel42 , this is because the ARS and the builder are thought to hold only operators of the same order Check these two examples (using Vensim syntax with the translator). Note that in the first, as addition and subtraction are same-level operations, they are saved in the same ARS, but for the second case, as exponential and addition have different order, a new ARS is created.

>>> from pysd.translators.vensim.vensim_element import Component
>>> expr1 = Component("my_expr", ([], []), "1 * (2 + 3-4)")
>>> expr1.parse()
>>> expr1.ast
ArithmeticStructure(
    operators=['*'],
    arguments=(
        1,
        ArithmeticStructure(
           operators=['+', '-'],
           arguments=(2, 3, 4)
        )
    )
)
>>> expr2 = Component("my_expr", ([], []), "1 * (2 + 3^4)")
>>> expr2.parse()
>>> expr2.ast
ArithmeticStructure(
    operators=['*'],
    arguments=(
        1,
        ArithmeticStructure(
            operators=['+'],
            arguments=(
                2,
                ArithmeticStructure(
                    operators=['^'],
                    arguments=(3, 4)
                )
            )
        )
    )
)

This is built parsing the expression recursively and taking into account the operation order in each loop, check pysd/translators/vensim/parsing_grammars/components.peg and pysd/translators/vensim/vensim_utils.py:split_arithmetic and pysd/translators/vensim/vensim_element.py:EquationVisitor. Similar approach is done for XMILE models.