Opened 4 years ago
Closed 4 years ago
#6037 closed defect (fixed)
FMI2.0 Co-simulation: Defining stop time in setupExperiment causes whole simulation to run on the first co-simulation step.
Reported by: | Owned by: | Lennart Ochel | |
---|---|---|---|
Priority: | high | Milestone: | Future |
Component: | FMI | Version: | v1.14.1 |
Keywords: | Cc: |
Description
Thank you for all the great work!
I tried to make it easier to reproduce this problem.
I pinned this problem down to defining the parameter stopTime of the setup experiment function.
- Download the zip file attached. It contains the logs as well as the means to reproduce the problem.
- Optionally run the instructions in run.ps1.
In the attached file, there are two sets of logs: with and without stoptime.
The correct behavior (without stop time) produces the following output:
time,step-size,{simple}.simple.x
0.0,0.0,0.0
0.01,0.01,0.01
0.02,0.01,0.02
Given the differential equation in Simple.mo, x should be equal to time.
The incorrect behavior happens when stop time is defined:
time,step-size,{simple}.simple.x
0.0,0.0,0.0
0.01,0.01,0.02
0.02,0.01,0.02
where x is equal to the stop time of the co-simulation (0.02) after the first co-simulation step.
From the logs, the only difference is is the setupExperiment function:
INFO [main] (CoeInitialize.scala:405) - logFmi2Call OK simple fmi2SetupExperiment: toleranceDefined=0 tolerance=0 startTime=0 stopTimeDefined=0 stopTime=0
INFO [main] (CoeInitialize.scala:405) - logFmi2Call OK simple fmi2SetupExperiment: toleranceDefined=0 tolerance=0 startTime=0 stopTimeDefined=1 stopTime=0.02
Attachments (1)
Change History (16)
by , 4 years ago
Attachment: | om_setup_exp_issue.zip added |
---|
comment:1 by , 4 years ago
I was unable to attach the jar files that run the co-simulation. Email me if you need those. I hope the logs will be enough to understand what's going on.
comment:2 by , 4 years ago
The problem seems to be here:
https://github.com/OpenModelica/OpenModelica/blob/master/OMCompiler/SimulationRuntime/fmi/export/openmodelica/fmu2_model_interface.c.inc#L1441
We should change it to something like:
tEnd = currentCommunicationPoint + communicationStepSize; if (comp->stopTimeDefined) tEnd = tEnd > comp->stopTime ? comp->stopTime : tEnd;
The FMI specification is not quite clear what the FMU should do if stopTimeDefined is set to true. See:
https://github.com/modelica/fmi-standard/issues/474
https://github.com/modelica/fmi-standard/issues/680
As a workaround for previous versions of the OpenModelica one could just edit the provided fmu2_model_interface.c.inc
and change:
if (comp->stopTimeDefined) tEnd = comp->stopTime; else tEnd = currentCommunicationPoint + communicationStepSize;
to the proposed fix.
I'll make a PR for this to see if it breaks any of our tests.
comment:3 by , 4 years ago
Thank you for the quick reply Adrian.
From looking at the code you provided, it seems that tEnd is being used with two meanings:
Meaning 1: tEnd is the time at the end of the co-simulation step.
Meaning 2: tEnd is the time of the end of the co-simulation.
comp->stopTimeDefined defines tEnd with meaning 2.
The code that you showed defines tEnd with meaning 1.
The two meanings are not compatible.
As far as I understand the FMI standard, the FMU exporter can use the stopTime to pre-allocate resources and some housekeeping.
Since this is not done in Open modelica, I'd say that open modelica FMUs should just ignore the stop time.
So I think the fix to this issue is just to remove the lines:
if (comp->stopTimeDefined) tEnd = comp->stopTime; else tEnd = currentCommunicationPoint + communicationStepSize;
and add the following in their place:
tEnd = currentCommunicationPoint + communicationStepSize;
follow-up: 12 comment:4 by , 4 years ago
The question is what should we do if the simulator asks to go beyond the defined stopTime.
Should we just return fmi2Error
or as you say just ignore this fact completely?
comment:5 by , 4 years ago
From my side I vote for just ignoring the defined stopTime, I'll ask our FMI expert @lochel what
he thinks when he comes back from holidays. For now I'll implement your suggestion in a PR.
comment:6 by , 4 years ago
Hm, I looked more a the existing code and it seems we could use the defined stopTime to allocate structures for time events:
https://github.com/OpenModelica/OpenModelica/blob/master/OMCompiler/SimulationRuntime/fmi/export/openmodelica/fmu2_model_interface.c.inc#L651
comment:8 by , 4 years ago
Replying to adrpo:
As a workaround for previous versions of the OpenModelica one could just edit the provided
fmu2_model_interface.c.inc
and change:
Hi Adrian and Claudio,
thank you for dealing with this issue.
I have the same problem on my Ubuntu computer but I cannot find the fmu2_model_interface.c.inc
file that should be modified as a workaround.
Can you tell me where to find it?
Thank you for the help.
comment:9 by , 4 years ago
I think it is in /usr/include/omc/c/fmi-export/ but I'm not near a computer to check.
comment:10 by , 4 years ago
Yes, that is the place:
adrpo33@ida-0030:~$ ls /usr/include/omc/c/fmi-export/ fmu1_model_interface.c.inc fmu2_model_interface.c.inc fmu_read_flags.c.inc fmu1_model_interface.h fmu2_model_interface.h fmu_read_flags.h
comment:12 by , 4 years ago
Replying to adrpo:
The question is what should we do if the simulator asks to go beyond the defined stopTime.
Should we just returnfmi2Error
or as you say just ignore this fact completely?
The FMI standard says that fmi2Error should be returned in that case.
Btw, I'll be using github for issues from now on. Didn't know that that was the modern way to track the issues in open modelica :)
comment:13 by , 4 years ago
Yeah, that is what I implemented, returning fmi2Error if a doStep with a time above the defined stopTime is used.
Regarding Trac and GitHub issues we are in the process of migrating all the Trac database to github. When that is done this Trac will become read-only and we will only be using github.
comment:15 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I merged the PR and I will close the ticket. If there are any issue we can reopen it.
Logs