Skip to content

Enable saving and restoring full simulation state #777

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.

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

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

kyllingstad
Copy link
Member

@kyllingstad kyllingstad commented Nov 6, 2024

This closes #768. Some changes may warrant a bit of extra explanation:

execution and algorithm: I have taken one step towards prohibiting adding/removing sub-simulators after the co-simulation has begun, as decided in #771. In particular, I've added the execution::initialize() function, which marks the point where such changes are no longer allowed. (This is a backwards-compatible change, because it gets automatically called by execution::step() if it hasn't been called manually.)

slave_simulator: The changes in slave_simulator.cpp really ought to have been included in #769. Unfortunately, I didn't realise they were necessary before it was all put into the context of saving algorithm and execution state. Basically, I thought I could get away with just saving each FMU's internal state, but it turns out that we also need to save the slave_simulator "get" and "set" caches.

(For those interested in the nitty-gritties, this is due to some subtleties regarding exactly when the cache values are set in the course of a co-simulation, relative to when the values are passed to the FMU. At the end of an "algorithm step", the "set cache" is out of sync with the FMUs input variables, and won't be synced before the next step. Simply performing an additional sync prior to saving the state is not sufficient, because that could have an effect on the FMUs output variables, thus invalidating the "get cache". That could in principle be updated too, but then the slave_simulator is in a whole different state from where it was when we started to save the state.)

Copy link
Contributor

@davidhjp01 davidhjp01 left a comment

Choose a reason for hiding this comment

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

LGTM, just minor comments :)

serialization::node export_state() const
{
assert(references.size() == originalValues.size());
assert(std::all_of(modifiers.begin(), modifiers.end(), [](auto m) { return !m; }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Other option is to print warning if manipulators are currently applied and then continue exporting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this could be a good option too. The main problem is that I am very unsure of what will happen if manipulators are active – under which circumstances it will work just fine, when it will subtly fail, and when it will fail spectacularly. I suspect that it will produce wrong results in the majority of cases, so I thought it was safer to make it an assertion rather than a warning. I think the whole “modifier” concept should be completely redesigned with a proper interface (à la simulator, observer, etc.) for this to work. It is just too easy to hide statefulness and aliasing behind a std::function object.

Copy link
Member

Choose a reason for hiding this comment

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

It seems a natural simplification to disallow manipulated state for now, but I suspect we'll soon want to keep at least some simple modifiers active during state export/import. Most modifiers should be pretty "simple" - for example variable overrides setting a fixed value on the reference. So it would be nice to retain the option to test with modifiers on.

We keep track of the modified references, so one option could be to include them in the warning. Assuming the owner of the simulation knows why the modifiers were set and what effect they have on the system. Maybe logging this is good practice even when state export isn't involved, but then it's not clear who, if anyone, will pay attention to this warning.

Maybe starting by improving the modifier API is a good idea - how would we improve it? Provide API functions for"simple" modifiers like overrides?

@davidhjp01
Copy link
Contributor

davidhjp01 commented Nov 22, 2024

Just leaving some ideas here - I have used nlohman_json before and I liked it, and seems it has support to encode/decode json data to/from CBOR. May be worth to consider switching it from boost ptree? Also I like its documentation :) https://json.nlohmann.me/api/basic_json/

Update
There are some limitations using nlohman_json for CBOR encoding/decoding it is documented here: https://json.nlohmann.me/features/binary_formats/cbor/, but does not seem critical for our use case?

@kyllingstad kyllingstad merged commit 9037906 into dev/state-persistence Nov 25, 2024
20 checks passed
@kyllingstad kyllingstad deleted the feature/768-cosim-state-export branch November 25, 2024 19:40
davidhjp01 added a commit that referenced this pull request Mar 20, 2025
* Enable saving and restoring subsimulator state (#765)

This is the first step towards closing #756.  I've added functions
corresponding to FMI 2.0's `fmi2{Get,Set,Free}FMUstate()` throughout the
various layers of subsimulator interfaces and implementations:

* `cosim::slave` and its implementation in
   `cosim::fmi::v2::slave_instance`
* `cosim::simulator` and its implementation in `cosim::slave_simulator`

This led me to also remove the `slave_state` and `state_guard` stuff
that was in `slave_simulator.{hpp,cpp}`. The overloading of the "state"
terminology became confusing, and it seemed like it was a lot of code
for very little gain. (It was supposed to be a check of correct API
usage, but I can't remember it ever actually catching a bug.)

* Enable exporting and importing subsimulator state (#769)

This is a follow-up to #765 and the second and final step to close #756.

Here, I've implemented functionality to export the internal state of
individual subsimulators in a generic, structured form, and to import
them again later.

This exported form is intended as an intermediate step before
serialisation and disk storage.  The idea was to create a type that can
be inspected and serialised to almost any file format we'd like.

The type is defined by `cosim::serialization::node` in
`cosim/serialization.hpp`.  It is a hierarchical, dynamic data type with
support for a variety of primitive scalar types and a few aggregate
types: strings, arrays of nodes, dictionaries of nodes, and binary
blobs. (Think JSON, only with more types.) It is based on
Boost.PropertyTree

* Enable saving and restoring full simulation state (#777)

This closes #768. Some changes may warrant a bit of extra explanation:

**`execution` and `algorithm`:** I have taken one step towards prohibiting adding/removing sub-simulators after the co-simulation has begun, as decided in #771.  In particular, I've added the `execution::initialize()` function, which marks the point where such changes are no longer allowed.  (This is a backwards-compatible change, because it gets automatically called by `execution::step()` if it hasn't been called manually.)

**`slave_simulator`**: The changes in `slave_simulator.cpp` really ought to have been included in #769. Unfortunately, I didn't realise they were necessary before it was all put into the context of saving algorithm and execution state.  Basically, I thought I could get away with just saving each FMU's internal state, but it turns out that we also need to save the `slave_simulator` "get" and "set" caches.

(For those interested in the nitty-gritties, this is due to some subtleties regarding exactly when the cache values are set in the course of a co-simulation, relative to when the values are passed to the FMU. At the end of an "algorithm step", the "set cache" is out of sync with the FMUs input variables, and won't be synced before the next step. Simply performing an additional sync prior to saving the state is not sufficient, because that could have an effect on the FMUs output variables, thus invalidating the "get cache". That could in principle be updated too, but then the `slave_simulator` is in a whole different state from where it was when we started to save the state.)

* Feature/779 state serialization (#780)

* Enable saving and restoring full simulation state

Closes #768

* Added tinycbor based serialization method

* Debug test

* Replaced tinycbor -> libcbor (ver 0.1)

* deserialization using cbor_reader

* Separated ctx to own struct

* Tag as std::optional in ctx

* State serialization test

* Removed comments

* Addressed Linux build issue

* Added BouncingBall reference model from modelica for adding byte vector serialization test

* Added BouncingBall license in README

* Added comments

* Deleted state_serialization_test.cpp

* Added libcbor as PRIVATE for `target_link_libraries`

* Controlling fixed precision for logging float point values (#775)

* Added an option to file_observer_config to set
fixed precision value.

* Fixed build failure

* Reading canGetAndSetFMUstate and canSerializeFMUstate from model description file

* Added transitive_libs for libcbor to avoid cmake error

* linking libcbor statically.

* Resetting time series observer upon state restore

* State saving support with proxyfmu

* Split proxyfmu from the save_state_test.

* Uses upgraded thrift/boost

* Fixed boost header issue

* Fixed boost header issue

* Fixed boost header issue

* Update dependency

* Updated action

* Upgraded libzip

* Upgraded libzip

* libzip deprecated func fix

* Observer fix

* Addressing comments
1. Defining exported sourced explicitly
2. Try to use shared libcbor.

* made libcbor PUBLIC in cmake (may require the consumer also declare libcbor for linking libcosim)

* Addressed comments

* Addressed comments

* Addressed comments - 02

* Addressed comments - 03

* Updated cosim_capabilities -> simulator_capabilities. Using cosim::serialization::format::cbor to choose which serialization/deserialization to use

* update

* pretty_print format added

* Updated error message

* Updated error message

* Updated error message

* Using fmu::model_description::capabilities in copy_current_state() and export_state()

* Updated error message

* Don't require importing def_types.h

* Using updated proxyfmu that hides thrift related libs

* Updated field names for the model description

* Updated proxyfmu dependency

---------

Co-authored-by: Lars T. Kyllingstad <lars.kyllingstad@sintef.no>

* Fixed an issue from the merge

---------

Co-authored-by: Lars T. Kyllingstad <lars.kyllingstad@sintef.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants