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

Define zero-crossing surfaces as variables #587

Closed
masoud-najafi opened this issue Jun 30, 2019 · 10 comments
Closed

Define zero-crossing surfaces as variables #587

masoud-najafi opened this issue Jun 30, 2019 · 10 comments
Assignees
Milestone

Comments

@masoud-najafi
Copy link
Collaborator

Instead of a using two APIs to retrieve zero-crossing surfaces and their number, individual zero-crossing surfaces can be defined as variables in FMI-3. Defining zero-crossing surfaces in this way presents several advantages:

  1. Get rid of fmi3Status fmi3GetEventIndicators(fmi3Component c, fmi3Float64 eventIndicators[], size_t ni);
  2. Get rid of fmi3Status fmi3GetNumberOfEventIndicators(fmi3Component c, size_t* nz);
  3. get rid of the "numberOfEventIndicators" attribute in the XML
  4. Each zero-crossing surface can have its own name and description which is useful in case of problem during the simulation, such as sliding model.
  5. Zero-crossing surfaces can be monitored individually, and may even be outputted at output ports of the FMU.

Designation of the variables that take part in the zero-crossing vector:
In the ModelStructure the subsection can be defined. This allows indicating the reference of variables that make the zero-crossing vector as well as the dependency of zero-crossing surfaces on other variables.

@chrbertsch
Copy link
Collaborator

Regular webmeeting:

Torsten S.: "zero crossing" is a term from Simulink, we typically use event indicator. We should use it consistentently.
Antoine: "zero crossing is a classical term from literature"
Andreas J: "events" is a wider definition. Zero crossings is a special case.
Torsten S.: only for state events we can use this kind of event indicators
Andreas J.: We should not change the names compared to older FMI versions, if there is no reason.
Andreas J: In essence it is removing functions and adding a subsection in the model structure.
Torsten S: I think we should leave it as it is (float64 variables have other attributes)
Pierre: It would not add attributes.
Torsten S.: We should further discuss it.
Andreas J: this was discussed for FMI 2.0. Does someone remember?
Massoud: I will search in the issues.
Christian: Massoud: Could you give an example for the new subsection?
Pierre: We have to check the interplay with the arrays. (E.g., changing number of state variables)
We need a completely worked out proposal

@chrbertsch
Copy link
Collaborator

For cross-reference: see #7

@chrbertsch
Copy link
Collaborator

@KarlWernersson : What is your opinion on this?

@masoud-najafi
Copy link
Collaborator Author

For the sake of simplicity, I do the necessary changes on an FMI-2.0 example. Here is the bouncingBall example, taken from FMUSDK. numberOfEventIndicators is removed and EventIndicators subsection has been added to the ModelStructure. Please see attached XML file.
The C part can be implemented with smething like this, where GetEventIndicators is no longer an FMI API. GetEventIndicators is implemented by the master with fmi2getReal.

fmi2Status GetEventIndicators(fmi2Component c, fmi2Real eventIndicators[], size_t ni) {
/*
comp->eventIndicatorReferences is the array containing the reference of variables designated as eventIndicators in modelDescription.xml
comp->NumberOfEventIndicators is the size of comp->eventIndicatorReferences.
*/
ModelInstance *comp = (ModelInstance *)c;
assert(ni== comp->eventIndicatorReferences);
return fmi2GetReal(comp, comp->eventIndicatorReferences, comp->NumberOfEventIndicators, eventIndicators);
}

modelDescription.xml.txt

@HansOlsson
Copy link
Contributor

HansOlsson commented Aug 26, 2019

I would like to add a few points - and I'm hesitant to add this feature.

  • As for terminology: "zero crossing" is a normal term from the literature, but to me it is based on a mischaracterization of the problem and thus I prefer if it isn't used. The original problem has e,g,, 'x>2' which is then mapped to the function 'x-2', and then we have get problems if this zero-crossing function starts at zero. But if we look at the original formulation a zero corresponds to 'x==2' which isn't an issue. Thus it is a more a matter of exposing the event-state (the model believes 'x>2') together with the event indicator ('x-2'). We should ensure that the state and indicator are updated together and are consistent.
  • Naming event indicators will in most cases just create dummy names that aren't helpful, and thus it risks information overload (I recall we have had such names hidden somewhere in Dymola without anyone noticing). When debugging you can always create a "name" in the debugging session.
  • Event indicators are not always active (if a change doesn't matter one might as well disable the event indicator). That complicates the dependency - and risks that to make things simpler they are always active, which risks a severe performance penalty (chattering for inactive event indicators can happen).

@beutlich beutlich changed the title Define zero-crossing surfaces as varaiables Define zero-crossing surfaces as variables Aug 26, 2019
@chrbertsch chrbertsch added this to the v3.0 milestone Aug 27, 2019
@chrbertsch chrbertsch modified the milestones: v3.0, future Sep 11, 2019
@chrbertsch
Copy link
Collaborator

Regular Design Webmeeing:
Torsten B.: This introduces a new feature. We are late with FMI 3.0. Do not see the point, why we should do this.
Massoud: New feature: only variable names, the rest is simplification
Torsten S.: Generally you can assign a name
Torsten S.: One could define a layered standard.
Torsten B.: We would have to introduce new causalities
Andreas P.: Could we introduce this at a later point of time?
Torsten B.: only in 4.0
Andreas P.: too late in FMI 3.0 development.
Torsten B.: The gain is not sufficient.

Decision: not do this in FMI 3.0

@chrbertsch
Copy link
Collaborator

chrbertsch commented Feb 26, 2020

FMI Design Meeting:
Klaus: During debugging this would be very helpful
Klaus: Do we add such new things into FMI 3.0?
Andreas: I would consider this as a cleanup
Masoud: One could currently define them as local variables
Torsten: The initial ideas of having separate functions for states, event event identicators the idea was that these are used by the solver, and not for communication with the outside
Klaus: in FMI 2.0 we added variables for states
Torsten B: In FMI 3.0 we added variables for time ...
Torsten B: We could even consider getting rid of the special functions ... and use get
Klaus: Currently the most easy thing would be to have local variables and reference them

What could/should we clean up

  • Event indicators listed as local variables (just like for states and derivatives)
  • List them in the model structure (like the derivatives)
  • possibly change the way to get the changed the event indicators Efficient handling zero-crossing surfaces #588

Bigger Change:

  • get rid of special access functions (setTime, getEventIndicators, getDerivatives, getStates, setStates)
    What this means:
    Puts the burden from the exporter to the importer
    Andreas: this would make the API simpler
    Klaus: You always send forth and back the value references. This would be a vary large change.

Poll: Shall we realize this issue #587 in FMI 3.0?

  • Event indicators listed as local variables (just like for states and derivatives)
  • List them in the model structure (like the derivatives)
  • possibly change the way to get the changed the event indicators Efficient handling zero-crossing surfaces #588

-Poll
Yes:
No:
Against:

@chrbertsch
Copy link
Collaborator

Poll: Shall we realize this issue #587 in FMI 3.0?
Event indicators listed as local variables (just like for states and derivatives)
List them in the model structure (like the derivatives)
possibly change the way to get the changed the event indicators #588
-Poll
Yes: 8
No: 0
Abstain: 6

@friedrichatgc : will create a PR

@chrbertsch
Copy link
Collaborator

We then have value references for everything.

Question: Shall we use fmi3GetXXX and fmi3SetXXX functions instead of the special functions
special access functions (setTime, getEventIndicators, getDerivatives, getStates, setStates)?

Discussion:
Torsten S: This could have a performance implication as one would have to do many more checks
(many): would be a very big change
Claudio: do we guaraintee that the user has only one way to sets such a value
Andreas J: We could increase efficiency if we would have lifetime #515 in FMI 3.1
Markus: We could keep the special functions for that. We could even more exploit internal strucutre

Poll:
Pro: 1
Con: 7
Abstain: 6

@chrbertsch chrbertsch added the PR_needed A pull request has to be crearted label Feb 29, 2020
friedrichatgc pushed a commit that referenced this issue Mar 5, 2020
Clarify the number of states/event indicators (#741)
Parts of this commit will be changed again when #587 gets implemented next.
friedrichatgc pushed a commit to friedrichatgc/fmi-standard that referenced this issue Mar 10, 2020
The current FMI 3.0 has a well defined variable (including a
valueReference) for all types of values, including states, state
derivatives and the independent variable (time), but not for event
indicators.
This commit fixes this.
Event indicators are listed now in ModelStructure and have therefore a
corresponding variable with a valueReference. Event indicators can be of
causality local and output (as states).
The c-API function for getting event indicators keep the same but
fmi3GetFloat64 can now also be used.
t-sommer pushed a commit that referenced this issue Mar 11, 2020
and remove "numberOfEventIndicators" attribute

solves #842
@chrbertsch chrbertsch removed the PR_needed A pull request has to be crearted label Mar 11, 2020
@chrbertsch
Copy link
Collaborator

fixed with #842

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

No branches or pull requests

4 participants