Short-term fix to Issue #236 - global timestepping now accounts for Method::schedule()
#294
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mabruzzo
force-pushed
the
method-schedule
branch
2 times, most recently
from
3d1ac2a to
8e716c9
Compare
…counts for Method::schedule() In the longer-term additional changes will be required to get ScheduleInterval working when using wall-clock time for outputs. Complications arise when trying to call Schedule::next for the schedules configured for Method objects. In the future, we will need to either: 1. refactor control flow so that the method only gets called once per PE per cycle 2. refactor internals of Schedule so that its okay to call Schedule::next multiple times on a given PE during a single cycle.
gregbryan
approved these changes
Jul 2, 2023
jwise77
approved these changes
Mar 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix is very important if you want to schedule the following actions at given simulation times:
"output"method)"output"method)Currently, we can only reliably schedule the above to occur at particular cycle numbers.
Prior to this PR, there are currently 2 problems that inhibit this from working.
dtwas not being restricted to coincide with the simulation times at which methods are scheduled to be executed. This issue has been fixed by this PR.next()method is never called onScheduleinstances associated any of theMethodobjects. This creates issues for schedules based on "simulation time" or "wall-clock time" that are implemented withScheduleInterval(the existing implementation requires knowledge of how many timesnext()has been called to determine the next "simulation/wall-clock" when a method simulation). This issue has not been addressed by this PR.In lieu of fixing Problem-2, this PR adopted a short-term fix that modifies the
ScheduleIntervalso that it ignores the number of timesnext()has been called when considering how scheduling with simulation-time (this is very similar to what it does when considering scheduling based on simulation-cycle). However, scheduling based on wall-clock time remains broken -- it will require a more comprehensive solution to Problem-2. While this short-term fix is fairly crude, it's better than nothing (the new output and checkpoint machinery effectively can't be used without this fix in a bunch of real-world simulations).Problem-2 requires a somewhat more involved solution.
next()method is currently implemented requires that it be called once per Processing Element at the end of the compute phase for each method's schedule object (for the methods that were scheduled to be executed during that cycle). To accomplish this, we would probably need to add a callback to a new method of theProblemobject to be executed at the end of the compute phase (after each block has executed each relevant method object).nextmachinery so that it can be called multiple times per cycle (to do this, we would probably need to record the current cycle, current simulation-time, and wallclock-time at the start of the cycle)...I'm leaving this to be addressed in the future...