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

Initial drafting of msmi support #333

Merged
merged 38 commits into from
Oct 1, 2019
Merged

Conversation

ljamt
Copy link
Member

@ljamt ljamt commented Aug 26, 2019

This is the outcome of a mob session before summer where we started exploring features from WP2. About time we make a PR.

CraneController.json CraneController.xml and KnockleBoomCrane.json KnockleBoomCrane.xml specify the configuration of what FMI variables are used to form plugs, sockets and bonds. These files can be treated as extentions to the modelDesrcription.xml and should probably be located in the FMU root.

mapping.json SystemStructure.xml is the proposed cse configuraion file where the connections of plugs and sockets are defined in addition to the rest of the configuration from the SSP. The schema for the SystemStructure is defined in SystemStructure.xsd

The configuration has been extended to include:

  • SumConnection configuration as the first example of an instance of a "advanced connection"
  • decimationFactor for the sub simulators to configure their time steps

Closes #334 - "Custom configuration format(s) used by CSE" (discussion)
Closes #341 - "Configuration for advanced connections"
Closes #273 - "Configuration support for multiple time step"
Closes #250 - "PoC for wp2 standard"

@ljamt ljamt added enhancement New feature or request discussion needed Let's have a discussion about this labels Aug 26, 2019
@markaren
Copy link
Contributor

markaren commented Aug 26, 2019

As you might expect, I would like to see YAML here as well. Sure, we can switch from JSON at a later time, but it will require more work the more JSON is used. Now it is used for this, the Java msmi stuff, scenarios and the new orchestration format. If everyone agree on using JSON, then I can live with that. But am I really the only one that feels it is not suited for this? Perhaps the inability to embed comments is the most prominent example of why another format should be preferred.

@lassebje
Copy link

YAML vs json. I think we should focus on ease of parsing for machines and ease of writing for machines. The format should also have good support for a schema language and validation, such as xml schema or json schema. Probably YAML has one as well. It should also be well suited for hierarchies. YAML is probably more rigged towards human readability and "writability". My point is that this format will probably not be edited by humans that much in the end, and in my opinion we could just as well have used XML, since this is the language of FMI elsewhere. Again XML hurts to read and write for humans, but is excellent for computers.

@markaren
Copy link
Contributor

markaren commented Aug 26, 2019

I'm not stuck on YAML either, but I'd love to see a discussion on this. JSON is starting to be used for alot of different things, and I have never seen an open discussion on this. I would also prefer XML over JSON as we already use it for FMI, SSP, DCP and log configuration.

@markaren
Copy link
Contributor

In order to not pollute this PR any further with discussions on which format should be used, I opened a seperate issue for discussion: #334

@ljamt
Copy link
Member Author

ljamt commented Aug 26, 2019

cpp_cse_config_parser_test fails in the "linux with docker" build:
Error: parse error - unexpected '}'; expected end of input

Anyone on linux that is able to reproduce?

@eidekrist
Copy link
Member

At this moment we look for the extended model description files in the same directory as OspSystemStructure.xml. Should we, in this PR, support bundling of OspModelDescription.xml inside of the FMU file? I would guess this involves moving the parsing code into importer.cpp or the fmu classes, and making the data available through the slave class similar to cse::model_description.

# Conflicts:
#	src/cpp/CMakeLists.txt
@kyllingstad
Copy link
Member

@eidekrist:

Should we, in this PR, support bundling of OspModelDescription.xml inside of the FMU file?

I propose that we leave this for a later PR (after the 0.5.0 release). I think having the XML files outside the FMUs will be the main mode of usage in the use cases for the time being.

@kyllingstad
Copy link
Member

The simulator_map type is declared in both cse_config_parser.hpp and ssp_parser.hpp. This causes errors if you include both headers in one translation unit. (Just tried to do this in CSE CLI now.)

@ljamt
Copy link
Member Author

ljamt commented Sep 30, 2019

The simulator_map type is declared in both cse_config_parser.hpp and ssp_parser.hpp.
Can we have the simulator_map declared in execution.hpp?

@hplatou
Copy link
Contributor

hplatou commented Sep 30, 2019

The simulator_map type is declared in both cse_config_parser.hpp and ssp_parser.hpp.
Can we have the simulator_map declared in execution.hpp?

Either that, or put it in a new cse_config.hpp?

@ljamt
Copy link
Member Author

ljamt commented Sep 30, 2019

The simulator_map type is declared in both cse_config_parser.hpp and ssp_parser.hpp.
Can we have the simulator_map declared in execution.hpp?

Either that, or put it in a new cse_config.hpp?

A new cse_config.hpp sounded correct. Other commonalities from the two parsers could be included here.

@hplatou hplatou changed the title Initial drafting of msmi support - IN PROGRESS Initial drafting of msmi support Oct 1, 2019
Copy link
Contributor

@hplatou hplatou left a comment

Choose a reason for hiding this comment

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

Go, go, go!

Copy link
Member

@eidekrist eidekrist left a comment

Choose a reason for hiding this comment

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

🚀

@ljamt ljamt merged commit 95012ad into master Oct 1, 2019
@ljamt ljamt deleted the initial-drafting-of-msmi-support branch October 1, 2019 10:10
@markaren
Copy link
Contributor

markaren commented Oct 2, 2019

My comment about putting the (optional) runtime specific entries into a DefaultExperiment tag were never addressed. I'd really like to see something like:

<DefaultExperiment startTime="0.0">
      <Algorithm name="fixedStep" stepSize="1e-4"></Algorithm>
</DefaultExperiment>

Which can be completely ommited btw, rather than:

<StartTime>0.0</StartTime>
<BaseStepSize>1e-4</BaseStepSize>
<Algorithm>fixedStep</Algorithm>

However, the parameters to the Algorithm would be algorithm specific, dunno how that should best be handled. Probably some annotations stuff. Then basically you can have the exact same layout in SSP.

@eidekrist
Copy link
Member

I'm not completely opposed to this. Let's continue the discussion in #404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needed Let's have a discussion about this enhancement New feature or request
Projects
None yet
7 participants