-
Notifications
You must be signed in to change notification settings - Fork 29
Rrfs support hi pr 3.1 #129
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
base: main
Are you sure you want to change the base?
Conversation
…uerto Rico domains. Includes new code and configuration files. Tested on WCOSS2.
donaldwj
left a comment
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.
This is still the code that was inital submitted for 3.1. I think the retroactive 3.0 update erased parts of this somehow. core/forcingInputMod had the 3.0 setup and 3.1 config files were missing
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.
Pull request overview
This PR adds support for RRFS (Rapid Refresh Forecast System) forcing data for three regional domains: North America (NA), Puerto Rico (PR), and Hawaii (HI). The changes introduce new input forcing options (keys 24-26), corresponding regridding functions, and configuration templates for each region.
- Adds three new RRFS regional forcing options with domain-specific configurations
- Implements time-handling and regridding functions for RRFS NA, PR, and HI domains
- Updates forcing input module to support new GRIB variables and output mappings
- Provides configuration templates for operational use on WCOSS systems
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 36 comments.
Show a summary per file
| File | Description |
|---|---|
| core/time_handling.py | Adds neighbor-finding functions for RRFS NA, PR, and HI domains to calculate temporal interpolation windows |
| core/regrid.py | Implements regridding functions for each RRFS domain with GRIB2-to-NetCDF conversion and spatial interpolation |
| core/forcingInputMod.py | Updates forcing definitions to add CPOFP variable and extend output mappings for new RRFS products |
| Template/template_forcing_engine.config | Documents new RRFS forcing options (24-26) in configuration template |
| Config/WCOSS/v3.1/template_forcing_engine_PuertoRico_RRFS_Short.config | New configuration for Puerto Rico short-range RRFS forecasts |
| Config/WCOSS/v3.1/template_forcing_engine_PuertoRico_RRFS_Analysis.config | New configuration for Puerto Rico RRFS analysis runs |
| Config/WCOSS/v3.1/template_forcing_engine_Medium_GFS-NDFD.config | Adds RegridWeightsDir configuration option |
| Config/WCOSS/v3.1/template_forcing_engine_Hawaii_RRFS_Short.config | New configuration for Hawaii short-range RRFS forecasts |
| Config/WCOSS/v3.1/template_forcing_engine_Hawaii_RRFS_Analysis.config | New configuration for Hawaii RRFS analysis runs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def find_rrfs_na_neighbors(input_forcings, config_options, d_current, mpi_config): | ||
| """ | ||
| Function to calculate the previous and after HRRR conus cycles based on the current timestep. |
Copilot
AI
Jan 7, 2026
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 docstring incorrectly refers to 'HRRR conus cycles' when this function is for RRFS NA. Update to 'RRFS NA cycles'.
| Function to calculate the previous and after HRRR conus cycles based on the current timestep. | |
| Function to calculate the previous and after RRFS NA cycles based on the current timestep. |
| :return: | ||
| """ | ||
| if mpi_config.rank == 0: | ||
| config_options.statusMsg = "Processing Conus HRRR Data. Calculating neighboring " \ |
Copilot
AI
Jan 7, 2026
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 status message refers to 'Conus HRRR Data' but this function processes RRFS NA data. Update to 'Processing RRFS NA Data'.
| config_options.statusMsg = "Processing Conus HRRR Data. Calculating neighboring " \ | |
| config_options.statusMsg = "Processing RRFS NA Data. Calculating neighboring " \ |
| default_horizon = 18 # 18-hour forecasts. | ||
| six_hr_horizon = 84 # 84-hour forecasts every six hours. | ||
|
|
||
| # First find the current HRRR forecast cycle that we are using. |
Copilot
AI
Jan 7, 2026
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.
Comment references 'HRRR forecast cycle' but should reference 'RRFS forecast cycle' for consistency with the function's purpose.
| err_handler.log_critical(config_options, mpi_config) | ||
| err_handler.check_program_status(config_options, mpi_config) | ||
|
|
||
| # Calculate the current forecast hour within this HRRR cycle. |
Copilot
AI
Jan 7, 2026
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.
Comment incorrectly references 'HRRR cycle' instead of 'RRFS cycle'.
| input_forcings.fcst_date2 = next_rrfs_date | ||
|
|
||
| # Calculate the output forecast hours needed based on the prev/next dates. | ||
| dt_tmp = next_hrrr_date - current_rrfs_cycle |
Copilot
AI
Jan 7, 2026
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.
Variable name 'next_hrrr_date' is inconsistent - should be 'next_rrfs_date' to match the RRFS context and the variable defined at line 764.
| dt_tmp = next_hrrr_date - current_rrfs_cycle | |
| dt_tmp = next_rrfs_date - current_rrfs_cycle |
| return | ||
|
|
||
| # Create a path for a temporary NetCDF file | ||
| input_forcings.tmpFile = config_options.scratch_dir + "/" + "NAM_NEST_TMP-{}.nc".format(mkfilename()) |
Copilot
AI
Jan 7, 2026
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.
Temporary file name uses 'NAM_NEST_TMP' prefix but this function handles RRFS HI data. Update to 'RRFS_HI_TMP' for consistency.
| input_forcings.tmpFile = config_options.scratch_dir + "/" + "NAM_NEST_TMP-{}.nc".format(mkfilename()) | |
| input_forcings.tmpFile = config_options.scratch_dir + "/" + "RRFS_HI_TMP-{}.nc".format(mkfilename()) |
| 'UGRD_10maboveground', 'VGRD_10maboveground', | ||
| 'APCP_surface', 'DSWRF_surface', 'DLWRF_surface'], | ||
| 'APCP_surface', 'DSWRF_surface', 'DLWRF_surface', | ||
| 'PRES_surface'], |
Copilot
AI
Jan 7, 2026
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.
Inconsistent array formatting compared to similar entries (24-26). Line 391 ends with a closing bracket and comma while lines 395, 399, and 403 continue the array definition. This entry (23) should follow the same pattern as 24-26.
| 'PRES_surface'], | |
| 'PRES_surface', 'CPOFP_surface'], |
| # 19 - HRRR GRIB2 Alaska production files | ||
| # 20 - ExtAna HRRR AK FE output | ||
| # 24 - RRFS NA | ||
| # 25 - RRFA HI |
Copilot
AI
Jan 7, 2026
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.
Corrected spelling of 'RRFA' to 'RRFS'.
| # 25 - RRFA HI | |
| # 25 - RRFS HI |
| # 25 - RRFA HI | ||
| # 26 - RRFS PR |
Copilot
AI
Jan 7, 2026
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.
Comment ordering is inconsistent - line 48 shows '25 - RRFA HI' (should be RRFS HI) and line 49 shows '26 - RRFS PR', but this is a Puerto Rico config file using InputForcings = [26]. The comment at line 47 shows '24 - RRFS NA'. Consider reordering or clarifying the documentation.
| # 25 - RRFA HI | |
| # 26 - RRFS PR | |
| # 25 - RRFS HI | |
| # 26 - RRFS PR (Puerto Rico; used by this configuration) |
| BDateProc = syyyymmdy0000 | ||
| EDateProc = eyyyymmdy0000 |
Copilot
AI
Jan 7, 2026
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.
Date format appears to be placeholder text ('syyyymmdy0000' and 'eyyyymmdy0000') rather than valid date format. Should be in YYYYMMDDHHMM format as noted in comment at line 107, e.g., 202412230000.
| BDateProc = syyyymmdy0000 | |
| EDateProc = eyyyymmdy0000 | |
| BDateProc = 202412230000 | |
| EDateProc = 202412231200 |
No description provided.