-
Notifications
You must be signed in to change notification settings - Fork 45
Add SetStepDuration and TimeAdvanceModes #284
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Konrad Breitsprecher <[email protected]>
bdb9ebe to
7a4364e
Compare
| auto it = _otherNextTasks.find(otherParticipantName); | ||
| if (it != _otherNextTasks.end()) | ||
|
|
||
| auto minDuration = std::chrono::nanoseconds::max(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure using the minimum of the other sim. step durations works correctly if not all sim. step sizes are multiples of the smalles sim. step size? Especially if the 'minimal-duration' participant performs a hop-on and lands on an 'odd' current time.
Example with two participants A and B, the numbers are the 'now' timestamps, ordered as they would occur in SIL Kit (same x position means these steps don't wait for each other):
A: 0 2 4 6 8 10 12 14 16 18
B: 0 3 6 9 12 15 18
A participant C that uses the mimimum-duration mode added to the next run of that simulation, would do the same as participant A. Is this the desired behavior?
A: 0 2 4 6 8 10 12 14 16 18
B: 0 3 6 9 12 15 18
C: 0 2 4 6 8 10 12 14 16 18
To the best of my knowledge, if the participant C uses hop-on and joins a running simulation the following could happen (if C joins such that it's first now is taken from B):
A: 0 2 4 6 8 10 12 14 16 18
B: 0 3 6 9 12 15 18
C: 9 11 13 15 17
My initial feeling is that the selected duration should be computed as the difference between the 'current time' (of the minimal-duration participant) and the earliest 'next time' across all other 'normal' participants?
This would lead to the following in the starting-together case:
A: 0 2 4 6 8 10 12 14 16 18
B: 0 3 6 9 12 15 18
C: 0 2 3 4 6 8 9 10 12 14 15 16 18
And the hop-on case:
A: 0 2 4 6 8 10 12 14 16 18
B: 0 3 6 9 12 15 18
C: 9 10 12 14 15 16 18
Notice that they behave the exact same way in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @VDanielEdwards, you're right. Only adding the min. duration can lead to unaligned 'grids'. I will change the calculation such that all participants configured with ByMinimalDuration are guaranteed to trigger the SimStep with the same time.
622b8b5 to
c0ea946
Compare
…mestamps of all other sync. participants; Add Integration Tests; Remove automatic creation of TimeSynService in SimTestHarness Signed-off-by: Konrad Breitsprecher <[email protected]>
2d373b5 to
8b6c6d4
Compare
This PR adds support for dynamic time step management in the SIL Kit orchestration layer by introducing
SetStepDurationandTimeAdvanceModefeatures. The changes enable participants to either advance time based on their own step duration or synchronize with the time points among all participants.Key Changes:
TimeAdvanceModeenum with two modes:ByOwnDurationandByMinimalDurationSetPeriodmethod toSetStepDurationand exposed it in the public APICreateTimeSyncServicethat accepts aTimeAdvanceModeparameterWith these changes, a participant can reduce the step duration in a critical phase where a more fine grained synchronization is needed.