open-simulation-platform / libcosim

OSP C++ co-simulation library
https://open-simulation-platform.github.io/libcosim
Mozilla Public License 2.0
55 stars 10 forks source link

Bug in doStep ? #673

Closed markaren closed 2 years ago

markaren commented 2 years ago

https://github.com/open-simulation-platform/libcosim/blob/237178e6aec4dc6f741cda43b5db43e517c0eb2e/src/cosim/algorithm/fixed_step_algorithm.cpp#L208

stepCounter is incremented between two for loops that checks its value. This has to be wrong? Would lead to data not being transferred at the correct step?

kyllingstad commented 2 years ago

No, I'm pretty sure it is correct. If we didn't increment the step count between them, then all simulators would be expected to finish their individual step within one "group" step.

Here is an example of how it's supposed to go, assuming we have two simulators A and B with decimation factors 1 and 2, respectively:

  1. Start: stepCounter_ = 0.
  2. First algorithm::do_step() call:
    1. Since stepCounter_ % info.decimationFactor == 0 for both A and B, both get their do_step() functions called. (Now note: only A is supposed to finish within the current time step. B needs two steps.)
    2. Increment stepCounter_ to 1
    3. Since stepCounter_ % info.decimationFactor == 0 only for A, we only add A to the list of finished simulators and later transfer its variables.
  3. Second algorithm::do_step() call:
    1. Since stepCounter_ % info.decimationFactor == 0 only for A, only A gets its do_step() function called. (B is still working on the step that was initiated earlier.)
    2. Increment stepCounter_ to 2
    3. Since stepCounter_ % info.decimationFactor == 0 for both A and B, we add both to the list of finished simulators and later transfer both simulators' variables.
  4. ...and so it goes on.
markaren commented 2 years ago

Aight, got it. Seems valid.