Skip to content

Conversation

@rruusu
Copy link

@rruusu rruusu commented Oct 21, 2025

These changes are cherry picked from the branch "work", which includes changes originally made to the master branch in October 2024. I hadn't had the time earlier to rebase my code on the major refactoring done to SystemSC.cpp on 19.11.2024 (6d7a397).

Note:

  • This request is based on the maintenance/v2.1 branch, on which I have been working on.
  • This request includes the changes introduced by Output all CVODE steps #1522 and can be directly merged after it.

Related Issues

Purpose

This change request fixes some issues in simulation of strongly coupled systems. Some calls to FMUs are currently in the wrong order, so that fmi2_completedIntegratorStep() is called before all the inputs have been set to reflect the current state variable values and the time is updated for all components.

This request also adds support for

  • Performing events requested by fmi2_completedIntegratorStep() in CVODE.
    • Only adds a TODO comment to doStepEuler().
  • Skipping CVODE solver resets for events in which state derivatives are unchanged.

This change enables use of some models from PLECS in a model exchange system.

Approach

Calls to updateInputs() are moved to a time before the call to fmi2_completedIntegratorStep().

Includes some elimination of unnecessary FMI calls for models with no states or event indicators.

@CLAassistant
Copy link

CLAassistant commented Oct 21, 2025

CLA assistant check
All committers have signed the CLA.

@rruusu rruusu mentioned this pull request Oct 21, 2025
@rruusu
Copy link
Author

rruusu commented Oct 24, 2025

I will make a force push on this branch, which rebases it to #1522 and adds some new commits.


fmus[i]->doEventIteration();

fmistatus = fmi2_enterContinuousTimeMode(fmus[i]->getFMU());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation: This cannot happen before the updateInputs(eventGraph) below, as setting discrete inputs is forbidden in the continuous time mode.

// set time
for (const auto& component : getComponents())
component.second->setTime(time);
component.second->setTime(event_time);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops.

}

// Update all inputs, including non-continuous ones
updateInputs(eventGraph);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are not in continuous time mode.

@rruusu
Copy link
Author

rruusu commented Oct 24, 2025

I was wondering why I don't get a test failure for ./simulation/EventTest.lua on my local set-up.

I hadn't even realized that its's not even configured to run in an ucrt64 Windows build:

-- linux: yes
-- ucrt64: no
-- win: no
-- mac: no

I seem to have messed up something in the CVODE event handling. I need to look into it.

@arun3688
Copy link
Contributor

-- linux: yes
-- ucrt64: no
-- win: no
-- mac: no

This says on which platform the tests can run on, so in case of failures you can copy the expected output from jenkins , if you feel the expected output is correct

refs OpenModelica#1330

It seems that simply calling CVode with mode CV_NORMAL does not work correctly to return at the given stopping time, if the previous call stopped due to a root.
rruusu and others added 8 commits October 29, 2025 14:56
- Some errors made in commit 4c3fe79.
- All discrete inputs should be updated before going back to continous time mode in any FMU.

refs OpenModelica#1326
Even though the inputs values shouldn't change in continuous time mode, calling setters for such inputs is against the FMI standard and may result in error responses from FMUs.

refs OpenModelica#1326
- Termination requests from completedIntegratorStep were not handled.
- Unnecessary change to System::time in event-related termination with CVODE.

refs OpenModelica#1515
refs OpenModelica#1516

It is necessary to reset the solver when the state variable values change.
@rruusu
Copy link
Author

rruusu commented Oct 29, 2025

Rebased branch to current state of #1522

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 this pull request may close these issues.

4 participants