Skip to content

Read forcings only when necessary#1055

Merged
TomMelt merged 5 commits into
developfrom
issue1054_read-forcings-only-when-necessary
May 5, 2026
Merged

Read forcings only when necessary#1055
TomMelt merged 5 commits into
developfrom
issue1054_read-forcings-only-when-necessary

Conversation

@joewallwork
Copy link
Copy Markdown
Contributor

@joewallwork joewallwork commented Mar 2, 2026

Read forcings only when necessary

Fixes #1054

Task List

  • Linked an issue above that captures the requirements of this PR
  • Defined the tests that specify a complete and functioning change
  • Implemented the source code change that satisfies the tests
  • Commented all code so that it can be understood without additional context
  • No new warnings are generated or they are mentioned below
  • The documentation has been updated (or an issue has been created to do so)
  • Relevant labels (e.g., enhancement, bug) have been applied to this PR
  • This change conforms to the conventions described in the README

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

  1. Assumes that the simulation begins at midnight.
  2. Assumes that timesteps are aligned with the hour.

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.

@joewallwork joewallwork self-assigned this Mar 2, 2026
@joewallwork joewallwork added enhancement New feature or request ICCS Tasks or reviews for the ICCS team labels Mar 2, 2026
Copy link
Copy Markdown
Collaborator

@timspainNERSC timspainNERSC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 74d7891. Similarly for TOPAZ in ac44e60.

@joewallwork joewallwork force-pushed the issue1054_read-forcings-only-when-necessary branch from 379bb4b to c314139 Compare March 5, 2026 11:40
@joewallwork
Copy link
Copy Markdown
Contributor Author

[Rebased on top of develop after merging #1051]

@joewallwork joewallwork requested a review from TomMelt March 26, 2026 16:55
@TomMelt TomMelt force-pushed the issue1054_read-forcings-only-when-necessary branch from cf6d362 to b450e26 Compare April 27, 2026 06:29
Copy link
Copy Markdown
Contributor

@TomMelt TomMelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joewallwork . It looks good to me

@TomMelt TomMelt force-pushed the issue1054_read-forcings-only-when-necessary branch from b450e26 to ef53708 Compare April 27, 2026 16:06
Copy link
Copy Markdown
Collaborator

@timspainNERSC timspainNERSC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, now

@TomMelt TomMelt merged commit a69c650 into develop May 5, 2026
9 checks passed
@TomMelt TomMelt deleted the issue1054_read-forcings-only-when-necessary branch May 5, 2026 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ICCS Tasks or reviews for the ICCS team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid reading forcings every timestep

3 participants