Skip to content

Hindcast from nwm-rte - #43

Merged
mxkpp merged 5 commits into
developmentfrom
maxkipp-hindcast-from-rte
Jul 22, 2026
Merged

Hindcast from nwm-rte#43
mxkpp merged 5 commits into
developmentfrom
maxkipp-hindcast-from-rte

Conversation

@mxkpp

@mxkpp mxkpp commented Jul 21, 2026

Copy link
Copy Markdown
  • Changes function run_hindcast by adding a new boolean parameter yield_realizations. When True, this function acts as a Generator of (built) RealizationBuilder instances, and in that mode does not actually run ngen on each realization built. This is leveraged by the following nwm-rte PR:

  • Changes param input_path (a string path to a .config file) to have type flexibility -- if provided as a string it will behave as it had before, but if provided as a InputConfig instance it will be used directly, bypassing the need for a .config file.

  • Exposes a new boolean param do_override_log_file_prefix on method preprocess, allowing callers to change the OS env var affecting the log file prefix without needing to interact with the OS env var directly. nwm-rte's hindcast workflow needs this since it runs multiple realizations within one docker run call.

mxkpp added 3 commits July 14, 2026 23:51
…o config file (original behavior) or instance of InputConfig (new behavior), in anticipation of being called by nwm-rte.
@mxkpp mxkpp mentioned this pull request Jul 21, 2026
21 tasks
@mxkpp
mxkpp requested a review from jswade-rtx July 21, 2026 20:19
Comment thread README.md
@@ -135,7 +133,7 @@ from nwm_fcst_mgr.forecast import run_hindcast

run_hindcast(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you update the README for hindcasting to include the new yield_realization arg?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated

Comment thread python/nwm_fcst_mgr/forecast.py Outdated
num_iterations,
cold_start_state=None,
yield_realizations: bool = False,
) -> None | Generator[int, None, None]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this hint be Generator[RealizationBuilder, None, None], since we yield rb? Where is the int coming from?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed type hint

@mxkpp
mxkpp requested a review from jswade-rtx July 22, 2026 02:50

@jswade-rtx jswade-rtx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested state load for hindcasting and parallel processing for hindcasting. Looks good to me.

@mxkpp
mxkpp merged commit e9a0388 into development Jul 22, 2026
8 checks passed
@mxkpp
mxkpp deleted the maxkipp-hindcast-from-rte branch July 22, 2026 18:08
@mxkpp
mxkpp restored the maxkipp-hindcast-from-rte branch July 22, 2026 18:41
@mxkpp

mxkpp commented Jul 22, 2026

Copy link
Copy Markdown
Author

Restored branch temporarily in order to mirror these changes to the development-pw branch as directed: #44

mxkpp added a commit that referenced this pull request Jul 23, 2026
@mxkpp
mxkpp deleted the maxkipp-hindcast-from-rte branch July 23, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants