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

Improve handling of CLI and annotation flags/options. #10418

Merged
merged 4 commits into from Mar 25, 2023

Conversation

mahge
Copy link
Contributor

@mahge mahge commented Mar 20, 2023

Improve handling of CLI and annotation flags/options.

  • Consider experiment annotation when using omc on the CLI.
  • Consider __OpenModelica_commandLineOptions annotation when using omc on the CLI.
  • Consider flag --newBackend when specified on the CLI.

These have required some restructuring of the translateModel workflow. There is still so much more that can be cleaned up and refactored to make more sense.

@mahge mahge added COMP/OMC/Frontend Issue and pull request related to the frontend COMP/OMC/SimCode COMP/OMC/Interactive Environment COMP/OMC/New Backend Issues and pull requests related to the new backend. labels Mar 20, 2023
@mahge mahge self-assigned this Mar 20, 2023
@mahge mahge changed the title Minor fixes Improve handling of CLI and annotation flags/options. Mar 20, 2023
  - Consider `experiment` annotation when using omc on the CLI.
  - Consider `__OpenModelica_commandLineOptions` annotation when using omc on the CLI.
  - Consider flag `--newBackend` when specified on the CLI.

  These have required some restructuring of the translateModel workflow.
  There is still so much more that can be cleaned up and refactored to make
  more sense.

  Fixes OpenModelica#7860.
  Fixes OpenModelica#8122.
  Fixes OpenModelica#10142.
flatString := NFFlatString;
elseif not runSilent then
(dae, funcs) := NFConvertDAE.convert(flatModel, funcTree);
flatString := DAEDump.dumpStr(dae, funcs);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the new backend we could possibly use FlatModel.toString instead and avoid converting to DAE, since I assume we don't have any test cases that check the flat output with the new backend anyway. But it probably doesn't matter much.

Copy link
Contributor Author

@mahge mahge Mar 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since I assume we don't have any test cases that check the flat output with the new backend anyway

Probably not. We can try it.

However, I think it is better to do that where the NF creates the valid flat Modelica string now. That is, the NF should generate some flat output based on the options it is given and return either the valid or normal flat Modelica code. Somewhere around here if possible. Then this can be changed to check if there is some output string from the NF and print that. OR even better the NF itself can dump it, although that can perhaps require a bit more adjustment in other places.

@mahge mahge enabled auto-merge (squash) March 25, 2023 10:30
@mahge mahge merged commit 38b6c06 into OpenModelica:master Mar 25, 2023
@mahge mahge deleted the minor_fixes branch March 25, 2023 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP/OMC/Frontend Issue and pull request related to the frontend COMP/OMC/Interactive Environment COMP/OMC/New Backend Issues and pull requests related to the new backend. COMP/OMC/SimCode
Projects
None yet
2 participants