lbl-srg / modelica-json

Modelica to JSON Parser
Other
21 stars 17 forks source link

Check correctness of JSON schema #237

Closed AntoineGautier closed 1 month ago

AntoineGautier commented 2 months ago

Parsing a block from MBL using commit 648b390 of modelica-json creates a JSON file with the following structure.

image

In contrast, the MLS defines composition as follows:

composition :
   element-list
   { public element-list
     | protected element-list
     | equation-section
     | algorithm-section
   }
   [ external [ language-specification ]
     [ external-function-call ] [ annotation-clause ] ";"
   ]
   [ annotation-clause ";" ]

And element as follows:

element :
   import-clause
   | extends-clause
   | [ redeclare ]
     [ final ]
     [ inner ] [ outer ]
     ( class-definition
       | component-clause
       | replaceable ( class-definition | component-clause )
         [ constraining-clause description ]
     )

This brings the following observations.

  1. element_sections does not exist in the grammar: why is it used in the schema?
  2. equation_section should not be nested under element_sections but directly under composition (an equation is not an element). The same observation holds for algorithm_section when it is present.
  3. Why have protected_element_list at a deeper level than element-list? Both should be directly nested under composition. The same observation holds for public_element_list when it is present.
AntoineGautier commented 2 months ago

@JayHuLBL Can you look into that?

JayHuLBL commented 1 month ago

@AntoineGautier With the extra level of element_sections, we would potentially keep the element orders. As in the code, we may have sections like:

    [element]
    [element]

protected
    [element]
    [element]

public
    [element]
    [element]

When parsing it, we can have

   [element-list]
   element_sections: [
        {
            "protected_element_list": ...
        },
       {
            "public_element_list": ...
        }
   ]

Overall, the major intention is to keep the element order, as it appears in the code. The same reason for the equation sections.

AntoineGautier commented 1 month ago

Since public element-list, protected element-list, etc. are sections of composition that:

  1. may be repeated zero or more times ({ } in the grammar), and
  2. must be stored in a structure that preserves order, and
  3. are at the same level as element-list in the grammar,

the best option is probably:

{
  "composition": [
    { "element_list": [ ] },
    { "public_element_list": [ ] },
    { "protected_element_list": [ ] }
    { "equation_section": [ ] }
  ]
}

But maybe the refactoring effort is too much. An option minimizing the refactoring effort would be to rename element_sections into optional_sections (short for "sections of composition that may be repeated zero or more times"). The main issue with the current structure is that fundamentally equations and algorithms are not elements of the class, see https://specification.modelica.org/maint/3.6/class-predefined-types-and-declarations.html#S5.p6 So it is uncanny to find these sections under element_sections.

AntoineGautier commented 1 month ago

Discussing with @JayHuLBL on 8/9 we agree that the structure proposed above (https://github.com/lbl-srg/modelica-json/issues/237#issuecomment-2275196719) would be the best fit to the Modelica grammar. However, the refactoring appears significant. We'll close this issue for now as it isn't a development priority.