Skip to content
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

RRFS fire weather grid: post processing and product generation tasks #181

Merged
merged 6 commits into from
Jan 8, 2024
Merged

RRFS fire weather grid: post processing and product generation tasks #181

merged 6 commits into from
Jan 8, 2024

Conversation

BenjaminBlake-NOAA
Copy link
Contributor

@BenjaminBlake-NOAA BenjaminBlake-NOAA commented Dec 28, 2023

DESCRIPTION OF CHANGES:

In this PR, logic is added to run the post-processing and product generation tasks for the RRFS fire weather nest. They are the last tasks that need to be added to rrfs-workflow for RRFSFW. The run_prdgen task was added to the non-DA xml file, and there was an error with the make_ics task that needs to be resolved using place=vscatter instead of place=excl. I am not sure why this make_ics error is now occurring, but it is reproducible. Once this PR is merged, we should be able to use the rrfs-workflow to run the RRFS fire weather nest. The workflow was tested successfully on WCOSS2 and Hera. The firewx_gridspecs code was added to the rrfs_utl repository last week via this PR. This source code is needed for the product generation task in order to create the wgrib2 grid specifications, so the hash number in Externals.cfg needs to be updated.

A couple of small PRs will be required in the future to make additional changes to support the fire weather nest, such as:

  • Adding the domain check function to the make_grid task. This function is used to determine if the fire weather nest falls inside the RRFS grid based on the center latitude and longitude points. This modification should be added to rrfs-workflow before we start using it for the RRFSFW real-time parallel.
  • In exrrfs_run_prdgen.sh, support was only added for fire weather grids over CONUS (refer to line 339 of exrrfs_run_prdgen.sh). Additional script changes will be required in order to support other domains (e.g. Alaska, Hawaii).

TESTS CONDUCTED:

  • On machines/platforms:
  • WCOSS2

  • Hera

  • Orion

  • Hercules

  • Jet

  • Test cases:

  • Non-DA engineering test
  • DA engineering test
  • Other sample scripts: ush/sample_configs/RRFSFW/config.rrfsfw.wcoss2.sh, config.rrfsfw.hera.sh

ISSUE:

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 2, 2024

@BenjaminBlake-NOAA, you add a new xml file to parm in this PR. Compared to FV3LAM_wflow_nonDA.xml, it has only one task run_prdgen more. Do you plan to add more tasks to FV3LAM_wflow_firewx.xml in the future? If not, I think it would be better to use one xml template file for non-DA and firewx. If you think the suffix nonDA is not appropriate for firewx, it would be good to replace _nonDA with _firewx or use any other common name because the 'non-DA' samples are used for testing. @MatthewPyle-NOAA, what do you think about this?

@BenjaminBlake-NOAA
Copy link
Contributor Author

@chan-hoo I do not plan on adding any more tasks for the fire weather grid besides run_prdgen. I did not know if modifying the non-DA xml would be an option, but I would be open to doing that instead. Keeping the suffix as nonDA is fine with me.

@MatthewPyle-NOAA
Copy link
Contributor

I would agree with @chan-hoo that if FV3LAM_wflow_nonDA.xml and FV3LAM_wflow_firewx.xml are nearly identical, we should try to get away with a single template for both.

@BenjaminBlake-NOAA
Copy link
Contributor Author

Sure that's fine, I can update the PR today. One minor change I had to make was for the make_ics job to resolve a "Cannot place all ranks on node list" error:
<!ENTITY NATIVE_MAKE_ICS "<native>-l place=vscatter</native>">
So I will have to add this to the non-DA xml too. Hopefully it won't be an issue, or maybe there is a better solution for the error.

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 2, 2024

@BenjaminBlake-NOAA, I had the same issue last week. I think we can change NATIVE_ALL as follows:

<!ENTITY NATIVE_ALL    "<native>-l place=scatter</native>">
<!ENTITY NATIVE_RUN_FCST    "<native>-l place=excl</native>">

The run_fcst uses a separate env variable.

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 2, 2024

Either way would be fine with me.

@BenjaminBlake-NOAA
Copy link
Contributor Author

Thanks @chan-hoo. I'll include the NATIVE_ALL change as part of this PR. Also, with Dogwood being unavailable until Friday, we may want to hold off on merging this PR until Friday or early next week if either of you want to test it on WCOSS2.

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 2, 2024

@BenjaminBlake-NOAA, if this PR works on Hera or Orion, I can approve it.

@BenjaminBlake-NOAA
Copy link
Contributor Author

Okay sure, I'll let you know once I make the changes and if I'm able to get it working on Hera.

@BenjaminBlake-NOAA
Copy link
Contributor Author

@chan-hoo @MatthewPyle-NOAA This PR works on Hera and is ready to be tested/reviewed. For testing on Hera, I noticed that this setting in the config.sh file does not take effect: TPP_RUN_FCST. I had to add tpp=2 to RESOURCES_RUN_FCST in the xml manually. We will primarily run RRFSFW on WCOSS2, so this is not a major issue.

Copy link
Contributor

@MatthewPyle-NOAA MatthewPyle-NOAA left a comment

Choose a reason for hiding this comment

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

One small question before I approve...

ush/sample_configs/RRFSFW/config.rrfsfw.wcoss2.sh Outdated Show resolved Hide resolved
@MatthewPyle-NOAA MatthewPyle-NOAA self-requested a review January 4, 2024 20:34
Copy link
Contributor

@MatthewPyle-NOAA MatthewPyle-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good from my perspective, but will wait to hear from @chan-hoo since it impacts more files now.

@chan-hoo
Copy link
Contributor

chan-hoo commented Jan 4, 2024

@BenjaminBlake-NOAA, make_sfc_climo failed:

/scratch2/NCEPDEV/stmp3/Chan-hoo.Jeon/test_firewx/logs/rrfsfw.20231111/18/

@BenjaminBlake-NOAA
Copy link
Contributor Author

@chan-hoo Oh shoot. I forgot that I had to add this line to the xml file to run make_sfc_climo for the fire weather grid on Hera:

<!ENTITY RSRV_SFC_CLIMO "<account>&SERVICE_ACCOUNT;</account><queue>&QUEUE_DEFAULT;</queue><partition>bigmem</partition>">

We'll need to use the big memory nodes on Hera for this job to create the sfc_climo files using high resolution datasets. Is creating a RSRV_SFC_CLIMO entity the best way to go about doing this? If so I can update the PR tomorrow.

@BenjaminBlake-NOAA
Copy link
Contributor Author

@chan-hoo The make_sfc_climo task should work now, I just tested it on Hera.

@MatthewPyle-NOAA MatthewPyle-NOAA merged commit 58f8642 into NOAA-EMC:dev-sci Jan 8, 2024
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.

Add fire weather functionality into rrfs-workflow
3 participants