Enable saving and restoring full simulation state #777
Conversation
| serialization::node export_state() const | ||
| { | ||
| assert(references.size() == originalValues.size()); | ||
| assert(std::all_of(modifiers.begin(), modifiers.end(), [](auto m) { return !m; })); |
There was a problem hiding this comment.
Other option is to print warning if manipulators are currently applied and then continue exporting?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
|
Just leaving some ideas here - I have used Update |
This closes #768. Some changes may warrant a bit of extra explanation:
executionandalgorithm: 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 theexecution::initialize()function, which marks the point where such changes are no longer allowed. (This is a backwards-compatible change, because it gets automatically called byexecution::step()if it hasn't been called manually.)slave_simulator: The changes inslave_simulator.cppreally 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 theslave_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_simulatoris in a whole different state from where it was when we started to save the state.)