-
Notifications
You must be signed in to change notification settings - Fork 84
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
Clarification for matching #1808
Conversation
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.
The non-normative text contains normative language and I think that some of those conditions must be declared as normative (e.g. forbidding existense of x and x[1] at the same time).
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.
ok, now. Cherry-on-top would be a link to the structured naming convention sections...
I'm a bit confused again: If this is scheduled for a 3.0.1, which it should IMHO, I don't see how we can make normative additions. It should just clarify that matching includes matching using structured naming conventions, if the FMU follows those (indicated in the modelDescription). I would also be weary of putting too much redundant info/restrictions on structured naming in this location because this would be redundant and potentially conflicting with the actual definition of structured naming. We do need some information to clarify intent, but have to be careful about it. I'm still on holidays, but will be back next week and have a more careful review/proposal at that time. |
I thought this is for 2.0.4 - @chrbertsch ? |
I'd prefer to have this PR rather 3.0.1, than in 2.0.4. It also applies to FMI 3. Currently there is no hint how arrays with matchinRule "sequence" are handled. Should be add it? Maybe this is a general issue, which is not related to this PR.
|
Well, the current proposal does not refere to "plug" or "sequence" but tries only to clarify "matching". |
The discussion came up for 2.0.4 and we shall clarify this tiopic there also. But as @CSchulzeTLK pointed out, this shall also be clarified for FMI 3.0 and thus I first stared a PR toward the main branch (FMI3), as it is not possible to create a PR towards different branches (main and v2.0.x). |
The current proposal is only a clarification, that "matching" shall include matching using structured naming conventions under certain conditions.
Which redundant info/restrictions should be removed? |
Currently it is a normative text. The added text should be moved down to the non-normative section below. E.g. after "_A tool may choose to connect terminals with a different or unknown The first sentence should include "may", and refer to the matchingRules "bus" and "plug":
The sequence matching rule just defines a sequence of terminal member variables. At the time we did not discuss how the array variables are handled. My understanding is, that a terminal with matchingRule "sequence" is equivalent to a 1D array, even though different data types are allowed. So I think the non-normative text should suggest a serialization following the definition of the FMI 3 C-interface. However, maybe @antvl can comment on this.
|
Design Meeting: Pierre: we have to define, what "matching" means. Pierre: we currently only define what should definitely be connected. They could connect different things. Christian B: Is there consensus to clarify that "matching" of arrays and naming convention scalar variable is up to the tools and a quality of implementation? Andreas: there are other problems due to unicode names Andreas: one could define additional more strict matching rules. --> Agreement to follow Pierre's suggestion https://github.com/modelica/fmi-standard/pull/1808/files#r982362920 Klaus: we should make it explicitely, that matching is not equality. Christian S: we have not clarified, how to deal with native arrays in sequence. Pierre: I will finilize my commit to the PR, please comment and approve |
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.
As discussed in todays FMI design web meeting
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.
I am not sure "notional" is a good pick, but I have no better proposal.
Draft to fix #1799 on FMI3 side according to Pierre's and Christian S's proposal.