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

Clarification for matching #1808

Merged
merged 11 commits into from Oct 12, 2022
Merged

Clarification for matching #1808

merged 11 commits into from Oct 12, 2022

Conversation

chrbertsch
Copy link
Collaborator

Draft to fix #1799 on FMI3 side according to Pierre's and Christian S's proposal.

Copy link
Contributor

@andreas-junghanns andreas-junghanns left a 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).

@chrbertsch chrbertsch changed the title First draft of clarification for matching Clarification for matching Sep 8, 2022
Copy link
Contributor

@andreas-junghanns andreas-junghanns left a 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...

@pmai
Copy link
Collaborator

pmai commented Sep 8, 2022

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.

@andreas-junghanns
Copy link
Contributor

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 thought this is for 2.0.4 - @chrbertsch ?

@CSchulzeTLK
Copy link
Collaborator

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.

[An array ModelVariable in a terminal with matchingRule "sequence" is inserted in the terminal sequence in the serialized form described in section 2.2.7.1 (https://fmi-standard.org/docs/3.0/#serialization-of_variables).]

@chrbertsch
Copy link
Collaborator Author

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 P

Well, the current proposal does not refere to "plug" or "sequence" but tries only to clarify "matching".
What is the expected behaviour for "sequence"? Does the current proposal fit there also?

@chrbertsch
Copy link
Collaborator Author

I thought this is for 2.0.4 - @chrbertsch ?

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).
I will create a separate PR for 2.0.4 once we have an agreement for FMI 3.0.

@chrbertsch
Copy link
Collaborator Author

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).

The current proposal is only a clarification, that "matching" shall include matching using structured naming conventions under certain conditions.

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.

Which redundant info/restrictions should be removed?

@CSchulzeTLK
Copy link
Collaborator

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 terminalKind, if the matchingRule matches." at the end of the same section.

The first sentence should include "may", and refer to the matchingRules "bus" and "plug":

If a TerminalMemberVariable or TerminalStreamMemberVariable is an array, "matching" for the matching rules "plug" and "bus" may include "matching based on structured naming convention" besides pure name-equality if the following conditions are fulfilled:

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.
I think adding this sentence/paragraph to the same non-normative section should be sufficient:

An array ModelVariable in a terminal with matchingRule "sequence" may be interpreted as adding the scalar elements in the serialized form described in section 2.2.7.1 (https://fmi-standard.org/docs/3.0/#serialization-of_variables).]

@chrbertsch
Copy link
Collaborator Author

Design Meeting:

Pierre: we have to define, what "matching" means.
Andreas: Aren't we deliberately vague? We want to help importers and humans to connect variables.
Pierre: We kind of do define it, but are unclear.
Andreas: We could connect integer with double variables; it is up to the importer to handle this.
Klaus: citing <...> .
Andreas: "matching" is not defined.
Klaus: "all variables must be present."
Andreas: in some cases it is better to allow things.
Pierre: we should at least explain it more explicitely.
Klaus: we should be clear, as we call it matching "rules"
Andreas: we defined the difference "bus" and "plug" between as complete "match" or partial "match"
Pierre: I would have made the only clarification, that tools can use the structured naming convention for matching. The rest is a quality of implementation issue.
Klaus: For me "matching" is equality of names.
Antoine: I agree with Klaus. Currently the spec only deals with "variable names"
Christian S: We would just enable that tools could use structured naming convention names for "matching".
We had removed "equality" and introduced "matching".

Pierre: we currently only define what should definitely be connected. They could connect different things.
Andreas: there may be other conditions (as units)
Klaus: in the non-normative text we explain this ... "Matching is not restricted by variability, causality or variable type. Example: A fixed variable may be connected to a tunable variable, a variable of type fmi3Float64 may be connected to a variable of type fmi3Int32. However, it is recommended that the variable types and variabilities are equal"
Pierre: The whole discussion started because people wanted to use a solution in FMI 2.0.4. ...
Simple: How to handle arrays and structure naming variables in terminals.
This is not yet clear to experts. We must clarify this.
Andreas: we could just extending
Pierre: we could just have a simple non-normative explanation, no enumeration.
Jeffry: everything that we de should be consistent with SSP. There is also an SSM file ....
Pierre: SSP does not deal with native arrays. It makes a distinction between names on the connector and names in the FMU. Even SSP is more flexible. And it might become more flexible to solve this issue.
Andreas: We should not too fine granularly define connection rules.
Antoine: ... it is up to the importer to establishes a connection an array with with structure naming variables.
Pierre: this is possible even for FMI 3.0
Klaus: we wanted to get rid of this mess in FMI 3.0, and now we have double the mess.
I thought that we supported only the native arrays in new concepts as terminals.

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
Pierre: Terminals by definition imply two FMUs. There cuould be dots or "Umlaute".
Klaus: why would you do this? Names are clearly defined.
Andreas: An importer might look for a loose matching. Could be semi-automatic. This is a great help.

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.
Klaus: it is not defined.
Pierre: could be treated in the implementer's guide.
Andreas: gutt-wise I am with Klaus, but the world is not so well-defined and we must allow the importers to handle different situations.

Pierre: I will finilize my commit to the PR, please comment and approve

docs/2_4_common_schema.adoc Outdated Show resolved Hide resolved
Copy link
Collaborator

@pmai pmai left a 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

docs/2_4_common_schema.adoc Outdated Show resolved Hide resolved
docs/2_4_common_schema.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@andreas-junghanns andreas-junghanns left a 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.

@chrbertsch
Copy link
Collaborator Author

We have two approving reviews, and I think we should merge this.
However meging is blockes due to pending change request by Pierre, and I do not see how to fix this.
@pmai or @t-sommer could you please have a look? Thanks!

@chrbertsch chrbertsch merged commit 926e143 into modelica:main Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backporting of the FMI 3.0 Terminals and Icons approach to FMI 2.0.4
4 participants