Run fmi2SetXXX function to initialize FMU start values

[davidhjp01]

According to FMI 2.0.4 spec (Section 4.2.4), start values are read during instantiated state via fmi2SetXXX . But libcosim does not run these functions in instantiated state and this makes some of FMUs exported (e.g. Simulink) not initializing values accordingly.

For example see below cosim_demo_app that runs an integrator whose initial condition is set via parameter Parameter.Integrator_x0:

The initial condition is not properly set via the parameter (i.e. not starting from 10).
On the other hand, fmpy properly initialize this value when set in the simulation setting:

To fix this, this PR added an additional function initialize_start_values() that runs before simulator::setup() in fixed_step_algorithm::cosim::impl::initialize()

Check state_init_test.cpp to observe different behaviours with the previous libcosim version

[davidhjp01]

I guess this PR contains some conflicts with #777, will need to update based on the merge :)

[kyllingstad]

The implementation looks good, and it's great to get this fixed. Correctness issues should always have high priority, IMO.

That said, I wonder if this really needs to be an entirely new interface function in simulator. Couldn't the things you do in the new simulator::initialize_start_values() function be done at the start of simulator::setup() instead, before it calls slave_->start_simulation()?

[kyllingstad]

variable_type and value_reference are just integers in disguise, and should therefore be passed by value rather than by reference.

[kyllingstad]

Also, we have the scalar_value alias which you can use instead of that variant.

[kyllingstad]

When adding an FMU, you should update tests/data/fmi2/README.md and possibly include a licence file, cf. #764.

[kyllingstad]

The functions are generally listed in the order they're supposed to be called, so this should come before setup().

[kyllingstad]

We haven't made reference to the FMI spec anywhere else in this interface, nor in any other of our general interfaces, because they are supposed to be agnostic with respect to the underlying binary interface. All FMI-specific stuff is hidden away in the cosim::fmi module. (I admit that we're still leaking FMI details like a sieve, but we haven't been this explicit about it before.) I suggest writing a more general and extensive description of what the function is supposed to do instead.

Also, as commented elsewhere, the functions are generally in call order, so this should go before setup().

[restenb]

LGTM

[restenb]

That said, I wonder if this really needs to be an entirely new interface function in simulator. Couldn't the things you do in the new simulator::initialize_start_values() function be done at the start of simulator::setup() instead, before it calls slave_->start_simulation()?

Agree. I've moved the functionality into simulator::setup() instead now. Briefly updated docstring to point out that this function is for instantiating the FMUs and entering initialization mode. Also followed up your code comments.

[restenb]

Builds are running into test failures, all of which are something like this:

terminate called after throwing an instance of 'std::out_of_range'
  what():  Variable with value reference 0 and type 

Seems this is originating in FMILibrary.

[kyllingstad]

terminate called after throwing an instance of 'std::out_of_range'
  what():  Variable with value reference 0 and type 

Seems this is originating in FMILibrary.

I don't think so. It is caused by a C++ exception (std::out_of_range), while FMI Library is a C library (and C doesn't have exceptions at all).

[kyllingstad]

Looks good to me, provided the cause of the exception is found and fixed.

[davidhjp01]

Hei thanks for the follow up @restenb and @kyllingstad

• Passing variable_type as value to filter to avoid dangling reference that causes the test fails.
• StateInitExample.fmu was created using FMIKit (https://github.com/CATIA-Systems/FMIKit-Simulink/blob/main/LICENSE.txt), do we need to update the license to Clause-2 BSD? (i.e. because we are building the fmu using the source code from FMIKit).
• Removed the pdb file in the tests directory.

[restenb]

Thanks for the fix. I completely missed that I had introduced this.

I was looking at the setup when your commit came in, wondering if our setup order was somehow incorrect. But by the way I read this, and the table on page 110 of the FMI 2.0.4 specification, this appears to be correct now. Variables are initialized between instantiate_slave and setup_experiment in the flow.

instantiated:
In this state, start and guess values (= variables that have initial = "exact" or "approx" and
variability ≠ "constant") can be set,

For calls to fmi2SetXXX:
for a variable with variability ≠ "constant" that has initial = "exact" or "approx"

I don't recall that we use this initial variable, so that means we are basically always assuming initial=exact?

[restenb]

Note that since we have users waiting on this fix on our side, I'm aiming to merge this & release libcosim tomorrow.

[davidhjp01]

Thanks for the fix. I completely missed that I had introduced this.

I was looking at the setup when your commit came in, wondering if our setup order was somehow incorrect. But by the way I read this, and the table on page 110 of the FMI 2.0.4 specification, this appears to be correct now. Variables are initialized between instantiate_slave and setup_experiment in the flow.

instantiated:
In this state, start and guess values (= variables that have initial = "exact" or "approx" and
variability ≠ "constant") can be set,

For calls to fmi2SetXXX: for a variable with variability ≠ "constant" that has initial = "exact" or "approx"

I don't recall that we use this initial variable, so that means we are basically always assuming initial=exact?

Yeah, I also remember reading this one. I think libcosim currently does not parse initial attribute for ScalarVariable in model description file. So I just skipped checking it for now.

[kyllingstad]

I don't recall that we use this initial variable, so that means we are basically always assuming initial=exact?

No, it's more that we don't need to worry about the initial attribute for the use cases we support. Per the spec, input variables don't have an initial attribute, and parameters can only have initial=exact. initial=approx is only used for output variables and calculated parameters which enter into algebraic loops that need to be resolved iteratively, which is something we don't handle. initial=calculated needs no special handling AFAICT.