modelica / fmi-standard

Specification of the Functional Mock-up Interface (FMI)
https://fmi-standard.org/
Other
274 stars 85 forks source link

Ambiguities and other minor issues. #898

Closed clagms closed 4 years ago

clagms commented 4 years ago

I'm collecting the following list as I work on https://github.com/modelica/fmi-standard/pull/895.

The following are small issues that need to be addressed, but I don't know exactly how.

    • [x] Clarify sentence [Only for source code FMUs, a change might be useful in some cases.].
    • [x] The sentence Additionally, they can be called in parallel provided the functions are called in different process/threads. contradicts with the first bullet of the list where it appears.
    • [x] It's not crystal clear which variant is referred to in the sentence macros are provided in fmi3Functions.h to build.
    • [x] In sentence The goal is that both textual and binary representations of FMUs are supported, it's not clear whose goal we refer to. The way functions are declared?
    • [x] I think the term target simulator should be replaced by FMU importing environment. Similarly, we should use the term FMU exporting environment when referring to the software that produces the FMU.
    • [x] It's not clear what cache in A typical FMU will cache computed results for later reuse. means. Which FMU interface does this refer to? Co-simulation? Model exchange?
    • [x] Either get rid of the sentence The inputs and outputs of these FMUs can be connected with direct feedthrough, or clarify which FMI interface it refers to. It's definitely not basic cosim.
    • [x] In sentence triggering execution of synchronous and asynchronous external events for all exposed model parts of a set of FMUs, it's not clear why are we referring to a set of FMUs. There's a lot of implicit information in this sentence. Are those FMUs all implementing the Scheduled Cosim interface? I think it's best is we keep the description between a master algorithm and a single FMU, to be consistent with the other cosim interface descriptions.
    • [x] The sentence "A new caching technique allows a more efficient evaluation of the model equations than in other approaches." is too abstract to be included in the preamble. I see this reference to a "new caching technique" in many places of the standard, but am yet to find out where is it described, and why is it "new"?
    • [x] The sentence "These instantiation functions must be called successfully before any of the following functions can be called" is confusing because the functions following that sentence are the instantiation functions. Perhaps this sentence should be removed?
    • [x] Why is there a duplicate listing of the fmi3instantiate functions? Near "The Co-Simulation master signals to the FMU which features shall be used for the current instance of the FMU"
    • [x] In the sentence "For Hybrid Co-Simulation and Scheduled Co-Simulation or if any of the intermediate variable access modes is set to", it's not clear what the intermediate access modes are. There's no reference to where these are defined. In this case, I propose that either a reference is given for more details, or the sentence be expanded with more details.
    • [x] In the sentence "To provide more options to secure the code against unwanted preemption these callback functions are defined", it's not clear which code is referred to. I guess this refers to the FMU code, right?
    • [x] In the sentence "Even if the Co-Simulation master does not support preemption, at least an empty implementation of these callback functions must be provided to allow the reuse of code for different modes together with an efficient preemption" , it's not clear how an empty implementation of the callback functions leads to a reuse of code.
    • [x] Should "FMU" be used instead of "slave"?
    • [x] The sentences the variable defines an <<inputClock>> that is controlled by the master and the variable defines an <<outputClock>> that is controlled by the FMU. seem to be wrong, as they refer to the input and output values of the causality attribute. EDIT: Opened https://github.com/modelica/fmi-standard/issues/914.
    • [x] The following expressions all mean that the function fmi3GetXXX can be called on the corresponding variable, right? The variable value can be provided from another model or slave, allowed to use the variable value in another model or slave, The variable value can be used by another model or slave, cannot be used in connections. If this means the same, then we should replaced them with a single expression that specifies what can be done in terms of the API.
    • [x] There is a similar set of expressions being used to specific what can and cannot be set with fmi3SetXXX. These too needs to be made consistent.
    • [x] Some terms in the glossary show up as capitalized, while others show up in small case. Should this be made uniform?
    • [x] Clarify what the expression and document order in the XML attributes. means.
    • [x] Regarding the fmi3Discard status, one part of the doc says In Model Exchange, this return status is only possible for the functions: ... fmi3Get{VariableType}, and another part says For Model Exchange: fmi3Status = fmi3Discard is possible for fmi3GetFloat32 and fmi3GetFloat64 only. The later statement seems to be a refinement of the former. We should probably avoid this, as the reader might read the first statement only, and conclude that every fmi3Get{VariableType} function could return fmi3Discard.
    • [x] There is a section called 4.2.4. State Machine of Calling Sequence from Master to Slave in Co-Simulation Interface Types inside the basic co-simulation section. There is another state machine section in each of the other cosim interfaces. Is section 4.2.4. supposed to describe the state machines of each cosim interface types?
    • [x] The sentence For Co-Simulation FMUs, additional functions are defined in Section 4.2.1 to set and inquire derivatives of variables with respect to time in order to allow interpolation. seems misplaced. Section 2. is about defining what's common, but detailing what's specific to each interface type. This "section contract" should be applied on the whole section. TODO: Andreas agrees. Claudio will address this.
    • [x] I have trouble understanding the use of the bold style. For example, what is the aim of the bold elements in the following sentence "Clocks are defined for the evaluation of clocked model partitions and the related definition of suitable communication points." When should we use bold?
    • [x] The following sentence introduces a noun and a verb as terms: The term "clock" and "clocked" is therefore used for both clock types. Is this intentional?
andreas-junghanns commented 4 years ago
1. * [ ]  Clarify sentence `[Only for source code FMUs, a change might be useful in some cases.]`.

This sentence must go because it is not true: We have static binaries that can be linked as well where it is important that the platform types are transported and correctly linked into the simulator.

2. * [ ]  The sentence `Additionally, they can be called in parallel provided the functions are called in different process/threads.` contradicts with the first bullet of the list where it appears.

Remove "threads", leave only "processes".

3. * [ ]  It's not crystal clear which variant is referred to in the sentence `macros are provided in fmi3Functions.h to build`.

The macros are only needed for C-code compilation and/or static linking, solving link issues for multiple instances of a model. Dynamic linking does not require prefixes for function names. This is independent of the usage of global variables, see canBeInstantiatedOnlyOncePerProcess capability flag.

4. * [ ]  In sentence `The goal is that both textual and binary representations of FMUs are supported`, it's not clear whose goal we refer to. The way functions are declared?

See above. avoid "textual representation", should be "source code".

5. * [ ]  I think the term `target simulator` should be replaced by `FMU importing environment`. Similarly, we should use the term `FMU exporting environment` when referring to the software that produces the FMU.

Why not "importer" and "exporter" or "FMU importer" or "FMU exporter". Remove "environment" everywhere.

6. * [ ]  It's not clear what cache in `A typical FMU will cache computed results for later reuse.` means. Which FMU interface does this refer to? Co-simulation? Model exchange?

It refers to both. Both interfaces are written such that caching is possible, but not required.

7. * [ ]  Either get rid of the sentence `The inputs and outputs of these FMUs can be connected with direct feedthrough`, or clarify which FMI interface it refers to. It's definitely not basic cosim.

Proposal: Leave this as is: this is part of the guiding-ideas section. It is not specification text.

8. * [ ]  In sentence `triggering execution of synchronous and asynchronous external events for all exposed model parts of a set of FMUs`, it's not clear why are we referring to a set of FMUs. There's a lot of implicit information in this sentence. Are those FMUs all implementing the Scheduled Cosim interface? I think it's best is we keep the description between a master algorithm and a single FMU, to be consistent with the other cosim interface descriptions.

"Clocks" are the transport layer of how we synchronize "events" across FMUs. There is no need for the new clocks mechanism if you have only a single FMU, then 2.0 events are sufficient. In 3.0 the event synchronization of multiple FMUs is improved to avoid numeric dirt of floating point computations.

9. * [ ]  The sentence "A new caching technique allows a more efficient evaluation of the model equations than in other approaches." is too abstract to be included in the preamble. I see this reference to a "new caching technique" in many places of the standard, but am yet to find out where is it described, and why is it "new"?

Improve to: "Caching technique allow efficient evaluation of the model equations."

10. * [ ]  The sentence "These instantiation functions must be called successfully before any of the following functions can be called" is confusing because the functions following that sentence are the instantiation functions. Perhaps this sentence should be removed?

Easy to fix: remove sentence

11. * [ ]  Why is there a duplicate listing of the fmi3instantiate functions? Near "The Co-Simulation master signals to the FMU which features shall be used for the current instance of the FMU"

remove the function listing from 2.4.2 (refer to the listing above in 2.1.5), leave section "Multiple Co-Simulation Interface Support in one FMU" where it is but elevate all the same section levels to "numbered".

12. * [ ]  In the sentence "For Hybrid Co-Simulation and Scheduled Co-Simulation or if any of the intermediate variable access modes is set to", it's not clear what the intermediate access modes are. There's no reference to where these are defined. In this case, I propose that either a reference is given for more details, or the sentence be expanded with more details.

Insert the modes listed as parameter in the function above here again, e.g. "... intermediate variable access modes (intermediateUpdateTime, intermediateVariableSetAllowed, intermediateVariableGetAllowed) is set to true..." --- this needs verification with the hybrid Co-Sim group!!

13. * [ ]  In the sentence "To provide more options to secure the code against unwanted preemption these callback functions are defined", it's not clear which code is referred to. I guess this refers to the FMU code, right?

The code referred to here is the "model or controller code" inside the FMU.

14. * [ ]  In the sentence "Even if the Co-Simulation master does not support preemption, at least an empty implementation of these callback functions must be provided to allow the reuse of code for different modes together with an efficient preemption" , it's not clear how an empty implementation of the callback functions leads to a reuse of code.

Add non-normative rational: "This is included to avoid checking the function pointers for NULL in the model or controller code. A function call to a void-void function with an immediate return is hardly any overhead."

15. * [ ]  Should "FMU" be used instead of "slave"?

That depends on the text: clarify that a slave can be an FMU, but an FMU can also be a master of hierarchical FMUs again. "Slave" refers more to the communication aspect of the FMU (as in master and slave).

16. * [ ]  The sentences `the variable defines an <<inputClock>> that is controlled by the master` and `the variable defines an <<outputClock>> that is controlled by the FMU.` seem to be wrong, as they refer to the input and output values of the causality attribute.

These points have to be moved up to the appropriate input/output description inside the causality row of the table.

17. * [ ]  The following expressions all mean that the function fmi3GetXXX can be called on the corresponding variable, right? `The variable value can be provided from another model or slave`, `allowed to use the variable value in another model or slave`, `The variable value can be used by another model or slave`, `cannot be used in connections`. If this means the same, then we should replaced them with a single expression that specifies what can be done in terms of the API.

Seems like a bigger issue: I think these are different use cases for why you need to access the value of tha variable (read or write). The text tries to clarify what kinds of use-cases are allowed and not allowed. Let´s solve this later with more brains in the loop.

18. * [ ]  There is a similar set of expressions being used to specific what can and cannot be set with fmi3SetXXX. These too needs to be made consistent.

See above.

19. * [ ]  Some terms in the glossary show up as capitalized, while others show up in small case. Should this be made uniform?

Most terms are lower case now, it is easier now to get consistency by turning them all to lower case, unless they are well known abbreviations.

20. * [ ]  Clarify what the expression `and document order in the XML attributes.` means.

It might be clearer to write instead of "and document order in the XML attributes": "and the document order in the XML attributes for the respective dimensions".

21. * [ ]  Regarding the fmi3Discard status, one part of the doc says `In Model Exchange, this return status is only possible for the functions: ... fmi3Get{VariableType}`, and another part says `For Model Exchange: fmi3Status = fmi3Discard is possible for fmi3GetFloat32 and fmi3GetFloat64 only.` The later statement seems to be a refinement of the former. We should probably avoid this, as the reader might read the first statement only, and conclude that every fmi3Get{VariableType} function could return fmi3Discard.

Not for me: some numerics person must (TorstenB?) anwer this.

22. * [ ]  There is a section called `4.2.4. State Machine of Calling Sequence from Master to Slave in Co-Simulation Interface Types` inside the basic co-simulation section. There is another state machine section in each of the other cosim interfaces. Is section 4.2.4. supposed to describe the state machines of each cosim interface types?

Change heading.

23. * [ ]  The sentence `For Co-Simulation FMUs, additional functions are defined in Section 4.2.1 to set and inquire derivatives of variables with respect to time in order to allow interpolation.` seems misplaced. Section 2. is about defining what's common, but detailing what's specific to each interface type. This "section contract" should be applied on the whole section.

Agreed.

24. * [ ]  I have trouble understanding the use of the bold style. For example, what is the aim of the bold elements in the following sentence "Clocks are defined for the evaluation of **clocked model partitions** and the related definition of suitable **communication points**." When should we use bold?

Please remove all those "adornments" in the text. We are hunting and exterminating them whereever we can find them.

25. * [ ]  The following sentence introduces a noun and a verb as terms: `The term "clock" and "clocked" is therefore used for both clock types.` Is this intentional?

This needs to be clarified by the HCS group.

clagms commented 4 years ago

Thank you for the discussion @andreas-junghanns!

Here are some more:

    • [x] In the mathematical definition of clocked partitions, near the text "There are two kinds of evaluation modes:", is "evaluation" the same as "execution"? I think they are the same. If they are, then I think we need to move the definition of execution right to the beginning of section "2.1.8.1.1. Synchronous Clocks". This is because we say stuff like "A clocked partition is not executed during Initialization Mode", where the meaning of "execution" is still not clear. An example ambiguity is: can we do a partial evaluation of a clock (subactive or not) during initialization mode? EDIT: With the introduction, this is a bit clearer.
    • [x] In sentence, In most cases the signaling of an <<outputClock>> tick is a `strong event` that requires creating, I think we should remove the expression strong event. It's not used anywhere else, and its definition comes right after. There's also no mention of "weak event" in the document.
    • [ ] The sentence Additional cases need to be handled if a clock tick is not associated to an event that requires the creation of a communication point for the model partition that generated the event. is too abstract. Which cases are alluded to?
    • [ ] The meaning of the sentence Either <<output>> or <<input>>, a periodic <<clock>> ticks at equidistant sample time points that are known a priori (defined in `modelDescription.xml`) or are alternatively determined by the environment in case of <<inputClock,`input clocks`>>. is not clear. Can both input and output periodic clocks be fully determined in the modelDescription.xml?
    • [x] When using subscripts of more than one letter in the latex math environment, we have to use something that tells latex that the subscript is a whole word. For instance, $t{offset}$ renders as: image while $t{\mathit{offset}}$ renders as: image. EDIT: Opened https://github.com/modelica/fmi-standard/issues/915
    • [x] In the following sentence, can we just replace "For real-time computation use cases i.e. in Scheduled Co-Simulation..." by "In Scheduled Co-simulation, ...".
      For real-time computation use cases i.e. in Scheduled Co-Simulation the priority information is used also for task preemption configurations.
    • [x] The sentence "This is needed to allow for an external resource management to achieve an optimal control of model partitions." seems too abstract. What does "external resource management" mean? Is it the master algorithm?
    • [ ] The sentence If <<outputClock,`output clocks`>> and <<inputClock,`input clocks`>> are defined, it is possible to define a tick relationship from an <<outputClock>> to an <<inputClock>>. seems too ambiguous. Does it hold between clocks of the same FMU, or clocks of different FMUs?
    • [ ] There's no explicit explanation for the table below the sentence It is possible to define multiple unions of <<clock,`clocks`>>, with the maximum number defined by the number of available aperiodic <<inputClock,`input clocks`>>..
    • [ ] Same thing for the image under sentence A clock union can be defined based on the <<clockReference>> attribute of <<inputClock>> variables..
    • [ ] The table under The following table summarizes the use of the API functions by the environment for different kind of seems to be specific to Model Exchange. Yet it shows up in section 2.
    • [x] In some areas of the text, we use the double equality to describe a boolean expression (e.g., *FMUState == NULL. In other areas, we use a single equality (e.g, variability = discrete). Should we not stick to one notation? EDIT: When the inline code refers explicitly to C code, keep the double equality. (Rationale: the reader might get confused by reading what appears to be C code, but has a different meaning).
    • [x] The non normative text The simulation is restarted at this state, when calling <<fmi3SetFMUState>> with `FMUState`. seems misplaced, and does not add much information. I suggest removing it.
    • [x] I cannot make sense of the sentence The partial derivatives refer are defined as partial derivatives of certain variables of the FMU w.r.t. to other variables.
    • [x] In the sentence For co-simulation, this means to compute the partial derivatives at a particular communication point, I think that "co-simulation" should be "Co-simulation (xCS)", because I think the sentence applies to all cosim interfaces, right?
    • [x] I have trouble understanding the following non-normative sentence For example continuous-time <<input,`inputs`>> in *Continuous-Time Mode*.. It seems misplaced, and the sentence following it does not make it easier to understand. EDIT: Fixed in https://github.com/modelica/fmi-standard/pull/1157
    • [x] Why is section 2.1.12. Getting Number of Event Indicators part of section two, when it seems that event indicators are concept for the Model Exchange? EDIT: Leaving as is, as it is symmetric with the number of states and others, and don't know where to place it.
    • [x] Does the sentence (Note: If an array has more than one dimension the indices are serialized in the same order as defined for values), refer to row major order?
    • [x] The non normative text Note that elements `<ModelVariables>` and `<ModelStructure>` are mandatory, whereas `<UnitDefinitions>`, `<TypeDefinitions>`, `<LogCategories>`, `<DefaultExperiment>`, and `<VendorAnnotation>` are optional seems outdated because there are more optional elements in the figure above. I think the optional elements are clear in the figure... Shall we remove this sentence?
    • [x] The sentence In this section, the units of the variables are (optionally) defined. is confusing. First, it's non-normative text and I think it should be normative. Second, does it means that specifying the units of variables is optional, or does it mean that the section may or may not define the units of variables?
    • [x] I think the following non normative text should be normative.
      If no units are defined, element `<UnitDefinitions>` must not be present._
      _If 1 or more units are defined, this element must be present.
    • [x] The non normative text Depending on the analysis/operation carried out, the SI derived unit `rad` is or is not utilized, see discussion below. refers to a discussion that I could not find. Is it the next sentence? If so, why not just remove this sentence altogether?
    • [x] In the sentence These definitions are used as default values in variable elements, it's not clear what definitions are referred to.
    • [x] The sentence It is not allowed to set `strict` to `true` if `periodic = false` is confusing, as it is not clear where and how one would set strict = true. If the importing tool is not allowed to change the xml, why do we need this sentence?
    • [x] In the table near the sentence Log messages when returning, the code style is messed up.
    • [x] Section 2.2.7.1. Rationale seems to be non-normative text. Same for section 2.2.8.1. Rationale.
    • [x] The sentence <<structuralParameter>>: <<independent>> parameter (a data value that is constant during refers to <<independent>> why? Does structural parameter have anything to do with the independent variable?
    • [x] Sometimes we use the term (xCS) and sometimes we use the term Co-simulation. I think there is no difference between the two, as both seem to be used to denote Co-simulation (which in the context of the fmi standard), refers to the co-sim interfaces. Should we unify this? Maybe get rid of xCS? If xCS is not used often enough (I have the impression Co-simulation is used a lot more), we can just get rid of it to simplify things.
    • [x] The sentence Co-Simulation: By convention, the variable is from a differential seems incomplete.
    • [x] The list that starts with *fmi3Set{VariableType}* can be called on any variable with <<variability>>... seems to be misplaced. Doesn't it belong to the state machines of ME and Co-simulation?
    • [x] In the following sentence (and surrounding text), why are the latex formulas in blue? Does this have a special meaning? Are they different than the rest of the math used in the doc? Example: if_ latexmath:[\color{blue}{\frac{\text{ds}}{\text{dt}} = v,\ \frac{\text{dv}}{\text{dt}} =f(..)}]
    • [x] The text Ordered list of all event indicators, in other words, a list of value references where is not visible in the rendered html. I suspect that this is because there's a section being declared in inside a table column. Interesting that the adoc compiler does not complain...
    • [x] The way we explain the different attribute values in tables could be made more consistent. For instance, I like the list-like notation, where every item can start with `= dependent`: no particular structure,.... There are multiple places where this notation is already used. EDIT: Opened https://github.com/modelica/fmi-standard/issues/925
    • [x] I think the text Typical scenarios are to provide binaries only for one machine type should be non normative. Same for text The usual distribution of an FMU will be with DLLs/SharedObjects because then further automatic processing....
clagms commented 4 years ago
    • [x] The sentence In this way multiple event types and also <<outputClock>> ticks or interrupts seems to be in contradiction with the sentence coming immediately before it.
    • [x] I don't understand the goal of this sentence: The ratio between communication points created for a model partition and time can be seen as a model rate. In that sense multiple model partitions of a model define multiple model rates in a model.
    • [x] The sentence This Co-Simulation interface provides support for concurrent computation of model partitions seems misplaced. Is it a new list item?
    • [x] The paragraph started with An FMU can inform a Co-Simulation master that is able to provide intermediate output variables seems out of place. Intermediate Variable Access stuff is described in section 2.4.1. and this paragraph is in section 2.4.2.
andreas-junghanns commented 4 years ago
26. * [ ]  In the mathematical definition of clocked partitions, near the text "There are two kinds of evaluation modes:", is "evaluation" the same as "execution"? I think they are the same. If they are, then I think we need to move the definition of execution right to the beginning of section "2.1.8.1.1. Synchronous Clocks". This is because we say stuff like "A clocked partition is not executed during Initialization Mode", where the meaning of "execution" is still not clear. An example ambiguity is: can we do a partial evaluation of a clock (subactive or not) during initialization mode?

Open: Let´s ask the clocked partition people. Karl?

27. * [ ]  In sentence, `` In most cases the signaling of an <<outputClock>> tick is a `strong event` that requires creating ``, I think we should remove the expression `strong event`. It's not used anywhere else, and its definition comes right after. There's also no mention of "weak event" in the document.

Open: Synchronous clocks people need to tell us about strong and week events. We may have to point to the synchronous literature and point to their definitions. Karl?

28. * [ ]  The sentence `Additional cases need to be handled if a clock tick is not associated to an event that requires the creation of a communication point for the model partition that generated the event.` is too abstract. Which cases are alluded to?

Open: Agreed - this needs improvement. Karl?

29. * [ ]  The meaning of the sentence `` Either <<output>> or <<input>>, a periodic <<clock>> ticks at equidistant sample time points that are known a priori (defined in `modelDescription.xml`) or are alternatively determined by the environment in case of <<inputClock,`input clocks`>>. `` is not clear. Can both input and output periodic clocks be fully determined in the modelDescription.xml?

Open: I need to read this...

30. * [ ]  When using subscripts of more than one letter in the latex math environment, we have to use something that tells latex that the subscript is a whole word.
     For instance, $t_{offset}$ renders as:
     ![image](https://user-images.githubusercontent.com/871103/79337786-77959d00-7f26-11ea-9130-20c466748a48.png)
     while $t_{\mathit{offset}}$ renders as:
     ![image](https://user-images.githubusercontent.com/871103/79337707-4c12b280-7f26-11ea-8895-bb45b089d5d5.png).

This should go into a separate PR for sure! But: Agreed.

31. * [ ]  In the following sentence, can we just replace "For real-time computation use cases i.e. in Scheduled Co-Simulation..." by "In Scheduled Co-simulation, ...".

For real-time computation use cases i.e. in Scheduled Co-Simulation the priority information is used also for task preemption configurations.

This is only an example for the use of clocks. We also use them for HCS. This needs to be clarified here.

32. * [ ]  The sentence "This is needed to allow for an external resource management to achieve an optimal control of model partitions." seems too abstract. What does "external resource management" mean? Is it the master algorithm?

Open: I cannot say.

33. * [ ]  The sentence `` If <<outputClock,`output clocks`>> and <<inputClock,`input clocks`>> are defined, it is possible to define a tick relationship from an <<outputClock>> to an <<inputClock>>. `` seems too ambiguous. Does it hold between clocks of the same FMU, or clocks of different FMUs?

Always different FMUs. Question: Feedthrough of clocks? I do not think so! It would compete to the original source of the clock.

34. * [ ]  There's no explicit explanation for the table below the sentence `` It is possible to define multiple unions of <<clock,`clocks`>>, with the maximum number defined by the number of available aperiodic <<inputClock,`input clocks`>>. ``.

Open: Karl?

35. * [ ]  Same thing for the image under sentence `A clock union can be defined based on the <<clockReference>> attribute of <<inputClock>> variables.`.

Open: Karl?

36. * [ ]  The table under `The following table summarizes the use of the API functions by the environment for different kind of` seems to be specific to Model Exchange. Yet it shows up in section 2.

Open: HCS also has a Event Mode and SCS also uses clocks. We need to fix this table.

37. * [ ]  In some areas of the text, we use the double equality to describe a boolean expression (e.g., `*FMUState == NULL`. In other areas, we use a single equality (e.g, `variability = discrete`). Should we not stick to one notation?

TODO: Let´s use single = only - there are only 3 places with == right now.

38. * [ ]  The non normative text `` The simulation is restarted at this state, when calling <<fmi3SetFMUState>> with `FMUState`. `` seems misplaced, and does not add much information. I suggest removing it.

TODO: This sentence needs to be moved up a few paragraphs to where SetFMUState is explained.

39. * [ ]  I cannot make sense of the sentence `The partial derivatives refer are defined as partial derivatives of certain variables of the FMU w.r.t. to other variables`.

TODO: remove sentence.

40. * [ ]  In the sentence `For co-simulation, this means to compute the partial derivatives at a particular communication point`, I think that "co-simulation" should be "Co-simulation (xCS)", because I think the sentence applies to all cosim interfaces, right?

TODO: Fix both sentences, use Co-Simulation.

41. * [ ]  I have trouble understanding the following non-normative sentence `` For example continuous-time <<input,`inputs`>> in *Continuous-Time Mode*. ``. It seems misplaced, and the sentence following it does not make it easier to understand.

Open: Markus?

42. * [ ]  Why is section `2.1.12. Getting Number of Event Indicators` part of section two, when it seems that event indicators are concept for the Model Exchange?

Open: Discuss. I tend to agree - move?

43. * [ ]  Does the sentence `(Note: If an array has more than one dimension the indices are serialized in the same order as defined for values)`, refer to row major order?

TODO: Add "Event Indicators" and "States" to the interface type feature support table.

44. * [ ]  The non normative text `` Note that elements `<ModelVariables>` and `<ModelStructure>` are mandatory, whereas `<UnitDefinitions>`, `<TypeDefinitions>`, `<LogCategories>`, `<DefaultExperiment>`, and `<VendorAnnotation>` are optional `` seems outdated because there are more optional elements in the figure above. I think the optional elements are clear in the figure... Shall we remove this sentence?

TODO: Yes. The information can be read from the schemas etc. Here we might only introduce redundancies and conflicts.

45. * [ ]  The sentence `In this section, the units of the variables are (optionally) defined.` is confusing. First, it's non-normative text and I think it should be normative. Second, does it means that specifying the units of variables is optional, or does it mean that the section may or may not define the units of variables?

TODO: Agreed: cleanup sentence and make it normative.

46. * [ ]  I think the following non normative text should be normative.

If no units are defined, element <UnitDefinitions> must not be present._ _If 1 or more units are defined, this element must be present.

TODO: YES!!!

47. * [ ]  The non normative text `` Depending on the analysis/operation carried out, the SI derived unit `rad` is or is not utilized, see discussion below. `` refers to a discussion that I could not find. Is it the next sentence? If so, why not just remove this sentence altogether?

TODO: remove first sentence.

48. * [ ]  In the sentence `These definitions are used as default values in variable elements`, it's not clear what definitions are referred to.

TODO: Please propose an improve wording here.

49. * [ ]  The sentence `` It is not allowed to set `strict` to `true` if `periodic = false`  `` is confusing, as it is not clear where and how one would set `strict = true`. If the importing tool is not allowed to change the xml, why do we need this sentence?

TODO: change "it" to "the exporter".

TODO: also fix displayUnit and Item in "the" table.

50. * [ ]  In the table near the sentence `Log messages when returning`, the code style is messed up.

TODO: This needs CSS fixing. TorstenS?

51. * [ ]  Section `2.2.7.1. Rationale` seems to be non-normative text. Same for section `2.2.8.1. Rationale`.

TODO: this is normative (explains optional nature of ports). Fix heading to Overview.

52. * [ ]  The sentence `<<structuralParameter>>: <<independent>> parameter (a data value that is constant during` refers to `<<independent>>` why? Does structural parameter have anything to do with the independent variable?

TODO: fix independent

53. * [ ]  Sometimes we use the term `(xCS)` and sometimes we use the term `Co-simulation`. I think there is no difference between the two, as both seem to be used to denote Co-simulation (which in the context of the fmi standard), refers to the co-sim interfaces. Should we unify this? Maybe get rid of `xCS`? If `xCS` is not used often enough (I have the impression Co-simulation is used a lot more), we can just get rid of it to simplify things.

As long as it is in parenthesis, I would leave it as (xCS) to disturb the text less (expanding it would require more effort to read/skip).

54. * [ ]  The sentence `Co-Simulation: By convention, the variable is from a differential` seems incomplete.

Indeed, propose an ending... :P

55. * [ ]  The list that starts with `*fmi3Set{VariableType}* can be called on any variable with <<variability>>...` seems to be misplaced. Doesn't it belong to the state machines of ME and Co-simulation?

TODO: remove it all: this is explained below the state machines for each of the interface types. We need to make sure this all there. "there can only be one!" place where to list these restrictions.

56. * [ ]  In the following sentence (and surrounding text), why are the latex formulas in blue? Does this have a special meaning? Are they different than the rest of the math used in the doc? `Example: if_ latexmath:[\color{blue}{\frac{\text{ds}}{\text{dt}} = v,\ \frac{\text{dv}}{\text{dt}} =f(..)}]`

later...

57. * [ ]  The text `Ordered list of all event indicators, in other words, a list of value references where` is not visible in the rendered html. I suspect that this is because there's a section being declared in inside a table column. Interesting that the adoc compiler does not complain...

DONE: AndreasJ

58. * [ ]  The way we explain the different attribute values in tables could be made more consistent. For instance, I like the list-like notation, where every item can start with ```= dependent`: no particular structure,...``. There are multiple places where this notation is already used.

TODO: open another issue

59. * [ ]  I think the text `Typical scenarios are to provide binaries only for one machine type` should be non normative. Same for text `The usual distribution of an FMU will be with DLLs/SharedObjects because then further automatic processing...`.

Agreed: both non-normative. The first requires more content discussion (FMUs should be self-contained).

andreas-junghanns commented 4 years ago
60. * [ ]  The sentence `In this way multiple event types and also <<outputClock>> ticks or interrupts` seems to be in contradiction with the sentence coming immediately before it.

No, this is ok. Early Return is the poor man version of even handling.

61. * [ ]  I don't understand the goal of this sentence: `The ratio between communication points created for a model partition and time can be seen as a model rate. In that sense multiple model partitions of a model define multiple model rates in a model.`

TODO: someone explain this to us. Why is this sentence in HCS, not in SCS?

62. * [ ]  The sentence `This Co-Simulation interface provides support for concurrent computation of model partitions` seems misplaced. Is it a new list item?

TODO: Yes, needs to be on top-level again.

63. * [ ]  The paragraph started with `An FMU can inform a Co-Simulation master that is able to provide intermediate output variables` seems out of place. Intermediate Variable Access stuff is described in section 2.4.1. and this paragraph is in section 2.4.2.

TODO: Agreed: whatever is here and not said in 2.4.1 should move to 2.4.1. TODO: Early Return should get its own 2.4.2 as IVA has 2.4.1 now. And then the explanation here in 2.4.2 (currently) should then move there (future 2.4.2).

clagms commented 4 years ago

Thank you for the discussion and replies @andreas-junghanns!

andreas-junghanns commented 4 years ago

I merged #895 to prepare for tomorrow. Can we close this issue, or are still points in the TODOs here that are not separated out into their own issues?

clagms commented 4 years ago

@andreas-junghanns, I'm not sure we can close this issue already. I haven't addressed any of the above items. Have you addressed them?

andreas-junghanns commented 4 years ago

@andreas-junghanns, I'm not sure we can close this issue already. I haven't addressed any of the above items. Have you addressed them?

I did the point addressed to me - I edited the list to mark it DONE. I see all other issues in your hands - recruit anyone to help, open issues for larger problems to invite discussions. When all points here are addressed or moved to other tickets, you can close.

clagms commented 4 years ago

@andreas-junghanns, I did not address the following because Event Indicators is already in the table, and "States" would be supported by every cosim interface (which seems to kill the purpose of adding it to the table, right?).

TODO: Add "Event Indicators" and "States" to the interface type feature support table.

IZacharias commented 4 years ago

Regarding

    • [ ] In the sentence "For Hybrid Co-Simulation and Scheduled Co-Simulation or if any of the intermediate variable access modes is set to", it's not clear what the intermediate access modes are. There's no reference to where these are defined. In this case, I propose that either a reference is given for more details, or the sentence be expanded with more details.

Insert the modes listed as parameter in the function above here again, e.g. "... intermediate variable access modes (intermediateUpdateTime, intermediateVariableSetAllowed, intermediateVariableGetAllowed) is set to true..." --- this needs verification with the hybrid Co-Sim group!!

I think this is not correct. The intermediate variable access modes seem to be intermediateVariableGetRequired, intermediateInternalVariableGetRequired and intermediateVariableSetRequired (the arguments to fmi3InstantiateXXX)

clagms commented 4 years ago

The following expressions all mean that the function fmi3GetXXX...

Leave as it is.

    • [ ] Regarding the fmi3Discard status, one part of the doc says In Model Exchange, this return status is only possible for the functions: ... fmi3Get{VariableType}, and another part says For Model Exchange: fmi3Status = fmi3Discard is possible for fmi3GetFloat32 and fmi3GetFloat64 only. The later statement seems to be a refinement of the former. We should probably avoid this, as the reader might read the first statement only, and conclude that every fmi3Get{VariableType} function could return fmi3Discard.

@KarlWernersson : Replace fmi3Get{VariableType} by fmi3GetFloat32 and fmi3GetFloat64

    • [ ] In the mathematical definition of clocked partitions, near the text "There are two kinds of evaluation modes:", is "evaluation" the same as "execution"? I think they are the same. If they are, then I think we need to move the definition of execution right to the beginning of section "2.1.8.1.1. Synchronous Clocks". This is because we say stuff like "A clocked partition is not executed during Initialization Mode", where the meaning of "execution" is still not clear. An example ambiguity is: can we do a partial evaluation of a clock (subactive or not) during initialization mode?

@KarlWernersson , Use "evaluation" instead of "execution".

    • [ ] The sentence "This is needed to allow for an external resource management to achieve an optimal control of model partitions." seems too abstract. What does "external resource management" mean? Is it the master algorithm?

@IZacharias : "external resource management" means an the FMU importer that is running a scheduled co-simulation and needs to schedule the execution of model partitions.

clagms commented 4 years ago
    • [ ] In section "3.1.8.2.1. Output Clocks", the paragraph started with Since <<outputClock,`output clocks`>> are ticking based on model internal seems to refer only to Communication Point Clocks, and not Synchronous clocks.
clagms commented 4 years ago

Most of the above issues were fixed with https://github.com/modelica/fmi-standard/pull/913

andreas-junghanns commented 4 years ago
    • [x] We use all kinds of terms to mean the tool/code that imports an FMU. We should pick a small set of clearly defined terms that we consistently use throughout the document. Here is what I found so far: importer, target simulator, simulation environment, environment, calling environment, master, Co-Simulation master algorithm
andreas-junghanns commented 4 years ago

Can you allow me write access to your branch glossary_model_exchange? My pushes fail.

clagms commented 4 years ago

Can you allow me write access to your branch glossary_model_exchange? My pushes fail.

You've been invited to my fmi standard fork, and I've created a draft pull request for you to have access.

andreas-junghanns commented 4 years ago

Can you allow me write access to your branch glossary_model_exchange? My pushes fail.

You've been invited to my fmi standard fork, and I've created a draft pull request for you to have access.

That worked - thanks!

andreas-junghanns commented 4 years ago

This issue does not help. It mixes issues, many solved by now and it is hard to even make a single pass through. @clagms : how do you propose to proceed to not loose the open points?

clagms commented 4 years ago

@andreas-junghanns We can have a meeting, go over the open points once again, and either make them into issues, or close them.

chrbertsch commented 4 years ago

Ticket-viewing-meeting

Andreas: I just got lost ... How do we continue with this? Klaus: Could we split this in smaller tickets? Claudio: we already have done this for the big issues. I have to go through this again. I will go through it again, create smaller ones and close this big issues Andreas: Perhaps many topics have already gone away.

@clagms will go through this

clagms commented 4 years ago

I've done a new iteration, and fixed what I could in https://github.com/modelica/fmi-standard/pull/1157 Most of the stuff still open is regarding communication point clocks.