Expose directional derivative and input/output derivative APIs#15
Conversation
Add GetDirectionalDerivative, SetRealInputDerivatives, and GetRealOutputDerivatives virtual methods to SlaveInstance, replacing the previous stub implementations that always returned errors. Available for both FMI 1.0 and FMI 2.0. Fix ctest invocation for Conan 2: cmake.ctest() in Conan 2 requires cli_args as a list, not a positional string argument.
There was a problem hiding this comment.
Pull request overview
This PR exposes previously-stubbed FMI derivative-related APIs by adding virtual methods to cppfmu::SlaveInstance and wiring the corresponding FMI 1.0 and FMI 2.0 C-entrypoints to forward to the slave implementation. It also updates Conan 2 usage for cmake.ctest() and bumps the project version.
Changes:
- Add
GetDirectionalDerivative,SetRealInputDerivatives, andGetRealOutputDerivativesas virtual APIs oncppfmu::SlaveInstancewith default “not supported” implementations. - Replace FMI 1.0 / FMI 2.0 stub C functions for these derivative APIs with real forwarding implementations.
- Fix Conan 2
cmake.ctest()invocation and bump version to1.2.0.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version.txt | Bumps library version to 1.2.0. |
| fmi_functions.cpp | Implements FMI 1.0/2.0 derivative API entrypoints by forwarding to SlaveInstance methods with standard error mapping. |
| cppfmu_cs.hpp | Adds new derivative-related virtual methods to the SlaveInstance interface. |
| cppfmu_cs.cpp | Adds default “operation not supported” implementations for the new virtual methods. |
| conanfile.py | Updates Conan 2 cmake.ctest() call to use cli_args list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… 1.0 function Extend cs_test fixture to cover fmi2GetDirectionalDerivative, fmi2SetRealInputDerivatives, and fmi2GetRealOutputDerivatives (FMI 2.0) plus fmiSetRealInputDerivatives and fmiGetRealOutputDerivatives (FMI 1.0). Tests cover both the default 'not supported' error path and the success path with a simple linear derivative model in TestSlave. Remove fmiGetDirectionalDerivative from FMI 1.0 build since it is not part of the FMI 1.0 co-simulation standard. Add cs_test_fmi1 executable target for FMI 1.0 test coverage.
kyllingstad
left a comment
kyllingstad
left a comment
There was a problem hiding this comment.
Thanks! This mostly looks good, I just had a small question.
| if (vr[i] == 0) { | ||
| value_ = value[i]; | ||
| } else if (vr[i] == 1) { | ||
| derivativeSupported_ = (value[i] != 0.0); |
There was a problem hiding this comment.
Why use a real variable to set whether derivatives are supported? Using a boolean would be more natural and, as a bonus, broaden the test coverage to also include SetBoolean() and GetBoolean().
There was a problem hiding this comment.
Changed to suggested data type to broaden test coverage.
|
Also wondering if the CI failures are pointing to some needed improvements in our workflow. |
Replace the vr=1 Real control variable with a Boolean, which is more semantically natural for an enable/disable flag. This also exercises the SetBoolean and GetBoolean virtuals, which previously had zero test coverage. The vr=1 value reference now throws in SetReal/GetReal, extending the invalid value reference error path coverage.
Unfortunately, this is a quirk of our new conan remote that I know no robust solution to yet. Rerunning the failed jobs resolves the build failure. When I have figured out how to avoid it altogether, a solution will appear here too. |
Add GetDirectionalDerivative, SetRealInputDerivatives, and GetRealOutputDerivatives virtual methods to SlaveInstance, replacing the previous stub implementations that always returned errors. Available for both FMI 1.0 and FMI 2.0.
Also fix ctest invocation for Conan 2: cmake.ctest() in Conan 2 requires cli_args as a list, not a positional string argument.