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

Missing elements when parsing Boolean expressions #255

Open
AntoineGautier opened this issue Nov 19, 2024 · 4 comments
Open

Missing elements when parsing Boolean expressions #255

AntoineGautier opened this issue Nov 19, 2024 · 4 comments

Comments

@AntoineGautier
Copy link
Contributor

AntoineGautier commented Nov 19, 2024

Running
node ../modelica-json/app.js -f Buildings/Templates/Plants/HeatPumps/Interfaces/PartialHeatPumpPlant.mo -o json
with

  • modelica-json commit f0b714b (current master)
  • MBL commit 07dfbd5 (current master)

yields the following JSON object for the enable annotation of typPumChiWatPri_select1:

                                        "element_modification": {
                                          "name": "enable",
                                          "modification": {
                                            "equal": true,
                                            "expression": {
                                              "simple_expression": {
                                                "logical_expression": {
                                                  "logical_or": [
                                                    {
                                                      "logical_and": [
                                                        {
                                                          "arithmetic_expressions": [
                                                            {
                                                              "name": "typDis"
                                                            },
                                                            {
                                                              "name": "Buildings.Templates.Plants.HeatPumps.Types.Distribution.Constant1Variable2"
                                                            }
                                                          ],
                                                          "relation_operator": "=="
                                                        }
                                                      ]
                                                    }
                                                  ]
                                                }
                                              }
                                            }
                                          }
                                        }

Several parts of the Boolean expression used in the binding of the enable attribute are missing and the logical_or and logical_and clauses are not balanced:
https://github.com/lbl-srg/modelica-buildings/blob/master/Buildings/Templates/Plants/HeatPumps/Interfaces/PartialHeatPumpPlant.mo#L270-L278

      enable=(have_pumChiWatPriDed
      or have_chiWat and typArrPumPri==Buildings.Templates.Components.Types.PumpArrangement.Headered)
      and typDis==Buildings.Templates.Plants.HeatPumps.Types.Distribution.Constant1Variable2),

@JayHuLBL Can you look into this?
(This issue is surprising because such expressions are commonly used in MBL templates so I wonder why we did not detect that earlier. This may be a regression.)

[EDIT] This is not a regression: commit a46a361 (used in first release of ctrl-flow) yields the same JSON object for the enable annotation of typPumChiWatPri_select1.

@AntoineGautier
Copy link
Contributor Author

The raw-json for the enable annotation of typPumChiWatPri_select1 is attached.
typPumChiWatPri_select1.raw.json

There seems to be several issues.

  1. In https://github.com/lbl-srg/modelica-json/blob/master/lib/jsonquery.js#L1330 the function returns undefined if rel.rel_op is falsy. The term have_pumChiWatPriDed is then resolved as undefined as it is the unique term of a relation with "rel_op": "".
  2. For the other relations we also have "rel_op": "" which seems incorrect as we should have "and" or "or" operators, no?

@JayHuLBL
Copy link
Contributor

@AntoineGautier Would you please check if the branch issue255_booleanExp fixed the issue?

@AntoineGautier
Copy link
Contributor Author

AntoineGautier commented Dec 13, 2024

The raw-json output is fixed with issue255_booleanExp.

The json output still bundles some terms of the expression into a string, rather than a recursively parsed object (see below). But this could be addressed in another issue for good separation of concerns.

"element_modification": {
    "name": "enable",
    "modification": {
        "equal": true,
        "expression": {
            "simple_expression": {
                "logical_expression": {
                    "logical_or": [
                        {
                            "logical_and": [
                                {
                                    "arithmetic_expressions": [
                                        {
                                            "name": "(have_pumChiWatPriDed or have_chiWat and typArrPumPri == Buildings.Templates.Components.Types.PumpArrangement.Headered)"
                                        }
                                    ]
                                },
                                {
                                    "arithmetic_expressions": [
                                        {
                                            "name": "typDis"
                                        },
                                        {
                                            "name": "Buildings.Templates.Plants.HeatPumps.Types.Distribution.Constant1Variable2"
                                        }
                                    ],
                                    "relation_operator": "=="
                                }
                            ]
                        }
                    ]
                }
            }
        }
    }
}

@JayHuLBL
Copy link
Contributor

The following issue is more related to the issue #241 and #247. I will further look into it.

It seems that we would need to address the 3 issues at the same time and a good decision about at which point the simple_expression should be string.

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

No branches or pull requests

2 participants