Open
Conversation
joewallwork
added
enhancement
timspainNERSC
requested changes
Mar 4, 2026
Collaborator
timspainNERSC
left a comment
timspainNERSC
left a comment
There was a problem hiding this comment.
This will be fine. There is an assumption that the time step length will either evenly divide an hour or be a multiple number of hours that evenly divide the day. But this seems to be a reasonable assumption.
I'd recommend not printing the time string every time you want to check for fresh data, though.
Comment on lines
+83
to
+84
| if (tst.start.format(TimePoint::msFormat) == "T00:00Z") { | ||
| forcingState = ParaGridIO::readForcingTimeStatic(forcings, tst.start, filePath); |
Collaborator
There was a problem hiding this comment.
To avoid string parsing every time, I suggest getting the duration since the epoch in seconds and seeing whether it is the top of the hour:
(tst.start - TimePoint()).seconds() % 3600. == 0.
(This should work: if it doesn't that's a bug in the Time classes).
Contributor
Author
joewallwork
force-pushed
the
issue1054_read-forcings-only-when-necessary
branch
from
379bb4b to
c314139
Compare
Contributor
Author
|
[Rebased on top of |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Read forcings only when necessary
Fixes #1054
Task List
Change Description
Currently, if the ERA5 and TOPAZ modules are included then they are read every timestep. This is unnecessary given that ERA5 data are only available once per hour and TOPAZ once per day.
This PR makes it so that ERA5 forcings are only read at the top of the hour and TOPAZ forcings are only read at midnight.
Caveats
Are we happy with these assumptions? If so, I can add config checks to ensure that they are satisfied. If not then I will have to modify how the XIOS calendar is handled.
(This change is also required to enable ERA5 and TOPAZ forcings with XIOS.)
Test Description
XIOS tests are updated accordingly.
Documentation Impact
A note on the assumptions has been added to the XIOS doc page.