-
Notifications
You must be signed in to change notification settings - Fork 24
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
Bugfix in C decentralized execution with small after delays #280
Conversation
3d31d54
to
570efdc
Compare
ebe3956
to
6d04e2b
Compare
A question about this fix: With decentralized coordination, a federate should be able to continue to execute even if it gets completely disconnected from the network (no messages come in). It looks to me like this use of max_level_allowed_to_advance will prevent this? Also, if a connection between federates has a large after delay (and messages take a long time to arrive), won't this block the receiving federate until that (future) message arrives? |
No, the MLAA will be updated when the STP offset expires. See this procedure, which was mostly contributed by Anirudh some months ago.
I'm not sure what you mean. If there is a large after delay, the receiving federate is still able to keep its logical time arbitrarily close to the physical time minus the STP offset, even if it has not received a message over the connection yet. When I finish working on this PR, I'll update it with details about the nature of the bug that it fixes and I'll mark the PR ready for review. |
6d04e2b
to
5d9f57a
Compare
Looks like merging #279 resulted in a conflict on this PR... |
The bug is only apparent when there are connections with small delays of at least one microstep.
55966ed
to
6956651
Compare
It turns out that what we had in The problems in
|
As for non-zero delay connections, I believe that the STAA wait time should be adjusted down by the amount of logical delay. If the amount of delay is 0, of course, this makes no difference, so in this case, it will be treated the same as if there were no delay on the connection. |
@edwardalee, should we merge this? |
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.
This looks good, but I've raised some questions.
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.
I think we can go ahead and merge this. Thanks!!
The bug is only apparent when there are connections with small delays of at least one microstep.
I claim responsibility for having recently introduced this bug.
Thanks to @jackykwok2024 for helping to uncover this bug.