-
Notifications
You must be signed in to change notification settings - Fork 4
added combitable1ds example #36
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
base: main
Are you sure you want to change the base?
Conversation
chrbertsch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we explain the behaviour via and example. I think we must define the implicit slicing cor in the definition of the domain and co-domain.
We will not support CombiTable1Dv, right? (Nor CombiTable2Ds, nor CombiTable2Dv?)
see https://build.openmodelica.org/Documentation/Modelica.Blocks.Tables.html
docs/index.adoc
Outdated
| include::examples/terminalsAndIcons_CombiTable1Ds.xml[] | ||
| ---- | ||
|
|
||
| The `combiTable1Ds1.table` variable appears twice: once with `variableKind="org.fmi-standard.fmi-ls-struct.map.domain"` and again with `variableKind="org.fmi-standard.fmi-ls-struct.map.codomain"`, since both domain and codomain data are stored in the same variable. The `terminalMemberVariable`, `combiTable1Ds1.y`, is a 2D-array, since the Modelica code specified two outputs with `columns = {2,3}`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must be defined above and may not appear for the first time in the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good points! Let's discuss it at today's design meeting and afterwards I'll rework this PR
I think we start with a CombiTable1Ds since it is simpler and then we have at least something for CombiTables in the spec. After that we can include the more complex examples
Split CombiTable1Ds Example to a separate section "Maps represented by `CombiTable1Ds` " with definition and example. This defines a third terminalKind for CombiTable1Ds
|
I split the CombiTable1Ds Example to a separate section "Maps represented by |
Thanks Elmir. |
|
Hi Christian, thank you for reviewing! I started with the assumption that it should be under rectilinear as well but thought it would be better with a third terminalKind. I should have been more clear in the PR that I proposed we instead "go back" to having a separate terminalKind for Combitables, to not make the presentation too cluttered. But if keeping Combitables under Rectilinear section is more inline with what the group wants I can move it inside there instead. It's no issue to me! I pushed a new pr so please see if that fits better with the idea. The thing left to change is to better specify the domain/codomain variableKind to agree with combitables. Both under [#common-concepts] and the definition for rectilinear grids we specify that the variable needs to be one-dimensional but that is not inline with combitables |
added an example for rectilineargrid with combitable1ds, and uploaded two .xml's for it