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

clock dependency definition in model structure #1370

Closed
chrbertsch opened this issue Mar 25, 2021 · 16 comments · Fixed by #1375
Closed

clock dependency definition in model structure #1370

chrbertsch opened this issue Mar 25, 2021 · 16 comments · Fixed by #1375
Assignees

Comments

@chrbertsch
Copy link
Collaborator

(Singled out from #1368, referring to #1287 and #1328 )

TorstenS:
Currently we use backward definitions of dependencies (for derivatives, outputs, initial unknowns) i.e. we define which variables it depends on. What is the rationale to define it forward for Clocks? If there can only be one associated Clock for a variable, we could use an attribute clock="vr" like we do for derivatives.

Christian:
From https://fmi-standard.org/docs/3.0-dev/#ModelStructure:
"A clocked variable my depend on multiple clocks and may therefore be listed in multiple elements. [More rigorous importers requiring a variable to be dependent on a single clock can reject FMUs violating this restriction.]"

Wouldn't it be possible to merge the "ClockedState" and "Clock" elements of the model-structure to a new ClockedVariables elements, where all Clocks a variable depends on are listed, and being a clocked "state" (and having the "previous" value) is just an attribute?

@KarlWernersson
Copy link
Collaborator

No because,
They list different things.
the clock is different and only states: Value reference for the clock in questions, a list of all variables that gets updated when that clock ticks.

The clocked state list the clocked state, and its dependencies on other varaibles(previous, inputs) when it is computed, in the same way as for outputs/StateDerivatives/InintialInputs etc.

The original proposal was to have clock as an attribute in the variable, but since we allow multiple clock dependencies, the this was moved to the clock, which will be (possible) huge lists of all variables in a model. (as you were discussing before).

But

The event indicator attribute, this is is just a list of of singel elements of a value reference, it could definitely be merged as an attribute of a variable. The only reason I see to list it in modelStrucuture is to enforce exporter to expose the event indicators as variables and it is eay to check thaht that list consists with the numberOfEventIndicators, I don't see a reason to enforce this. (Am I missing something?). Personally I would just dump out a "z" array and point to that, as for us the event indicators don't have a proper variable that represents them.

To sum it up
EventIndicators and Clock has a different structure and a different meaning that (otuput/StateDerivatives/ClockedStates/InintialUnknonws)
If we could clean this up in a good way I am all for it.

@chrbertsch chrbertsch changed the title clock dependependency definition in model structure clock dependency definition in model structure Mar 25, 2021
@andreas-junghanns
Copy link
Contributor

When we discussed where to put the clock dependency information, we also thought about adding an attribute to each variable (including clocks themselves). Unfortunately, I cannot find the issue anymore where we placed the reasoning for the final decision. But what is clear:

  • the information is a sparce matrix with very different dimensions (few clocks, many variables)
  • which way to present the matrix is functionally irellevant
  • one variable may depend on multiple clocks (so it is n:m, not 1:m)
  • even clocks may depend on (other!) clocks
    Moving the information into each Variable element seems fine to me as well.

I do not remember who needed direct access to the EventIndicators. But as they are optional, can´t we just leave them as they are, even if they seem odd compared to the other ModelStructure elements?

t-sommer added a commit to t-sommer/fmi-standard that referenced this issue Mar 25, 2021
@t-sommer
Copy link
Collaborator

The event indicator attribute, this is is just a list of of singel elements of a value reference, it could definitely be merged as an attribute of a variable. The only reason I see to list it in modelStrucuture is to enforce exporter to expose the event indicators as variables and it is eay to check thaht that list consists with the numberOfEventIndicators, I don't see a reason to enforce this. (Am I missing something?). Personally I would just dump out a "z" array and point to that, as for us the event indicators don't have a proper variable that represents them.

If I remember correctly the reason for adding the <EventIndicator> element, was to provide an order of the elements inside the z array, which would not be possible by defining them via an attribute.

The reason for having them as variables (as opposed to making them available only via fmi3GetEventIndicators()) was that some tools might be able to use the dependency information for more efficient root finding.

@andreas-junghanns
Copy link
Contributor

The reason for having them as variables (as opposed to making them available only via fmi3GetEventIndicators()) was that some tools might be able to use the dependency information for more efficient root finding.

Isn´t that sufficient reason to leave this in the ModelStructure for now and move on?

@t-sommer
Copy link
Collaborator

I think @KarlWernersson's point is that this information is not really useful, so we could keep it the way it was in FMI 2.0.

The advantages of the "variable approach" are listed in #587 (comment).

@KarlWernersson
Copy link
Collaborator

@t-sommer
Well I don't really se the advantages,
We setup the whole modelStructrue for event indicators only so we can give them a name, unit etc. Why.
For debugging purposes you could just log that.
We don't have dependency information so I don't rely see what else we gain.
It is true however that it cannot easily be changed into model variables as one would lose the order information related to the array returned from getEventIndicatros. it would require an index in the model variables referencing order so I don't think it's a good solution.

But still I don't se the point of exposing them at all.
what can you do with the information? Since we don't have dependency we cant solve local loops etc.
Is it some especial case where the output is aliased as the event Indicator and we then get the dependecy information?
What am I missing?

@TorstenBlochwitz
Copy link

The intention of connecting event indicators to variable was for debugging reasons. Christian Schulze (TLK) proposed it and most of agreed to that. It allows to give event indicators reasonable names, which helps to find out, which part of the model caused a zero-crossing. This is really very helpful since robust, exact and fast finding of the crossing point is numerically challenging. Everything which helps a user (or a support engineer) to find the cause of for example a discontinuity sticking or something similar is valuable!

@TorstenBlochwitz
Copy link

Dependency information for detection of algebraic loop is not that important here, since event indicators are rarely directly connected to other entities.

@KarlWernersson
Copy link
Collaborator

@TorstenBlochwitz , but an event indicator is usually an internal variable based on a logical expression that in turn could be based on one or several variables.?

E.g.

Varaiable a,
Varaible b,
Varialbe c,

when (a>b+c)
do my event.
end

Wouldn't it be better to make some form of description string describing the expression rather than makgin it a varaible and giving it a name? What woudl a name do? Posibly point to a part of the model if utilizing dot notation

@TorstenBlochwitz
Copy link

The idea was to use a mechanism similar to the existing solution for derivatives. Also, for derivatives in Modelica not every time a variable exists in the model: der(a)=b+c. In this case a tool would have to generate a variable and give it an appropriate name and comment.
For event indicators a tool could do it in a similar way: introduce a new FMI variable with a helpful name and a much more helpful comment (for example: “Eventindicator: a-(b+c)”).

@t-sommer
Copy link
Collaborator

for example: “Eventindicator: a-(b+c)

In this case, wouldn't it be possible (and make sense) to also (optionally) list the dependencies in the <EventIndicator> element?

@KarlWernersson
Copy link
Collaborator

Yes but we should relay think on what we want.

A solution would be to push in the standard to use the description string to explain the event the logic behind the event indicators. How is a solution intended to look?
We need to describe this well in the standard.

We are not curently using the ideas of derivatives since we dont utilize dependecies (I am unsusre if it is a good idea for event indicators)
So what we end up with is a list of varaibles, that could or coud not be well described.
For derivatvives it is usualy a very clear releasion ship to its state.

For event indicators this relasionship is less clear.
We even have a specific namenign covneiton alowing "der(" as part of a name. Do we want somethign smimlar for evnet indicatrs?

P.S
I was on parental leave during the discussion so i missed it.
I just feel that the solution is a bit incomplete and I didn't fully understood what it want to solve an I am still not completely sure on how.

@chrbertsch
Copy link
Collaborator Author

Regular design meeting:

Karl: we should not put something in the xml that we do not really use.
Torsten: The original intention to support better debugging by names.
Karl: we are now discussing the dependencies.
Torsten: they are optional. We took effort to forbid things, I just removed things.
Karl: this is only for debugging. But should we have this in the model structure?
Torsten: I added also something about the dimensions.
Klaus: An event indicator is an unknown. For me it fits there and it can be useful. This can be useful, e.g. to know if they need to be re-calculated.
Pierre: the original idea to provide names to event indicators was to make them more debuggable. If we have the dependencies, we need a good definition.
Karl: can one call directional derivatives for event indicators?
Klaus: Did we open "pandora's box" when we made event indicators variables?

Open question: Shall we have the dependencies? And can we explain better, why we need them.

@KarlWernersson : I will comment
@TorstenBlochwitz , @CSchulzeTLK , @masoud-najafi : Could you please comment?

@masoud-najafi
Copy link
Collaborator

Having the ordered list of event indicator variables, allows reconstituting the eventIndicator vector. Since eventIndicator vector can be built by the importer, the API fmi3GetEventIndicators() becomes redundant. Note that, unlike fmi3GetDerivatives() where x and xd should be contiguous, the eventIndicator vector need not to be contiguous.
Also, as has been mentioned above, having an eventIndicator as a variable with "name" and "description", it is very useful for debug purposes, e.g., consider a sliding-mode case.

@TorstenBlochwitz
Copy link

The API fmi3GetEventindicators is not redundant! It is the same concept like for fmi3GetDerivitatives, fmi3SetContinuousStates… And I know tools, where event indicators are stored in contiguous vector. We discussed this point already for states and derivatives and I would propose to keep it as it is.

Regarding introduction of dependencies for event indicators: Why should this be Pandora’s box? I think it does not hurt to allow it. I cannot imagine a use case for directional derivatives in connection with event indicators. But it might enable tools, which know their models at an atomic level to provide these dependencies, which would allow event location algorithms of importers to set only variables on which event indicators depend, which then allows FMUs to do the respective computations only, which altogether could lead to a certain performance gain. FMUs are free not to provide this information, importers are free to ignore it. So why not introduce it and reuse the same concept we already have for outputs and derivatives? This would only be consequent.

@KarlWernersson
Copy link
Collaborator

@TorstenBlochwitz @masoud-najafi @t-sommer
Discussion continued in #1377

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 a pull request may close this issue.

6 participants