-
Notifications
You must be signed in to change notification settings - Fork 46
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
Suspend #75
Conversation
Current coverage is
|
dest_lp->suspend_event == event && | ||
// Must test time stamp since events are reused once GVT sweeps by | ||
dest_lp->suspend_time == event->recv_ts) | ||
{ |
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 the event pointer / timestamp checks sufficient to avoid issues with event ties in optimistic mode? Not sure what the semantics regarding ordering of ties are.
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.
Hi John - yes, absolutely - no other event will have both the same pointer and time stamp.
Note, the time stamp is used solely to check that the event has not been re-used since
the LP was suspended. That is you suspend an LP and it presists til the end of the simulation.
That event will be re-used many times -- in fact once GVT sweeps by, you know that LP
will be suspended til the end ..
Make sense ?
thanks for the code review ...
Chris
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.
That makes sense. Happy to help review, I was after all pestering for the feature :).
// unsuspend the LP | ||
dest_lp->suspend_event = NULL; | ||
dest_lp->suspend_time = 0.0; | ||
dest_lp->suspend_error_number = 0; |
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.
The suspend flag needs to be unset here - otherwise, a suspended lp always stays suspended (and future RCs are never processed).
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 for the catch John.
There was a case where you don't want to unset the flag.
Let me read the code and double check that this is not that
case..
thanks,
Chris
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.
Hi John - I just pushed the fix.
thanks,
Chris
So I pulled in your suspend flag fix and re-tested the codes suite. The new suspend code works as expected! |
Great news! Thanks, Christopher D. Carothers Director, Center for Computational Innovations e-mail: chris.carothers@gmail.com fax: (518) 276-4033tel:%28518%29%20276-4033From: John Jenkins [notifications@github.com] So I pulled in your suspend flag fix and re-tested the codes suite. The new suspend code works as expected! — |
closed #15 |
Use this functionality by calling tw_lp_suspend in your event handler. An example is: