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.

Sign up for GitHub

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

Jump to bottom

Align fmi2DoStep() break condition with FMI 3.0 #210

Merged
merged 1 commit into from
Mar 2, 2022
Merged

Align fmi2DoStep() break condition with FMI 3.0 #210

merged 1 commit into from
Mar 2, 2022

Conversation

t-sommer
Copy link
Collaborator

fixes #203

andreas-junghanns
andreas-junghanns reviewed Feb 23, 2022
View reviewed changes
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.

Did you change the < into a <= on purpose?
This might sound like nit-picking, but I think it is important to allow ending exactly on the next communication point, even if that happens rarely.
Your change forces to go beyond solver step + epsilon...

@t-sommer
Copy link
Collaborator Author

This was the change suggested by @PTaeuberDS.

@pmai
Copy link
Collaborator

pmai commented Feb 23, 2022

As I said in the issue right now, can someone explain under which criteria the FMI 3 behavior/condition is deemed to be correct? It seems to clearly violate the FMI spec.

@t-sommer
Copy link
Collaborator Author

@pmai, can you propose a suitable way to account for the numerical tolerance?

@t-sommer t-sommer force-pushed the 203-next-communication-point branch from 0a92fb3 to a4e76e9 Compare March 1, 2022 10:36
@t-sommer t-sommer force-pushed the 203-next-communication-point branch from a4e76e9 to 38b9f52 Compare March 2, 2022 07:54
@t-sommer t-sommer merged commit d75d666 into modelica:master Mar 2, 2022
@t-sommer t-sommer deleted the 203-next-communication-point branch March 2, 2022 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreas-junghanns andreas-junghanns andreas-junghanns approved these changes

@pmai pmai Awaiting requested review from pmai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Different Co-Simulation behavior in FMI 2.0 and FMI 3.0 FMUs
3 participants
@t-sommer @pmai @andreas-junghanns