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

Update calculation of time-averaged radiation variables #900

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

LarissaReames-NOAA
Copy link
Collaborator

Description

This PR fixes incorrect diagnostic output for fhzero-averaged radiation output variables. Presently, there is a significant "sawtooth" appearance to these fields (such as dswrf_ave and ulwrf_avetoa) when we expect smoother lines with jumps only at fhzero diagnostic reset times. Here is an example of the present (OLD) result and the result from the current PR (FIX) from a 24-hour S2S regression test forecast.

iTerm2 RaUnsB rads_fhzero6_fix

Issue(s) addressed

  • fixes a problem found in investigating ufs-weather-model/#1767

Testing

Ran full regression test suite on Hera. See log files here: /scratch1/NAGAPE/hpc-wof1/lreames/ufs-weather-model/tests/logs/log_hera

Forecast output can be found here: /scratch1/NCEPDEV/stmp2/Larissa.Reames/FV3_RT/rt_4109245

All regression tests fail as sfcf*.nc files, as well as post-processed grib2 files as a result, are not identical. New baselines will be necessary.

Dependencies

  • waiting on noaa-emc/ccpp-physics/pull/241

Requirements before merging

  • All new code in this PR is tested by at least one unit test
  • All new code in this PR includes Doxygen documentation
  • All new code in this PR does not add new compilation warnings (check CI output)

@grantfirl
Copy link
Collaborator

@LarissaReames-NOAA Is there still work going on in this PR, or is there another reason that it is in draft mode? Also, I'm not seeing an associated ufs-weather-model PR.

@LarissaReames-NOAA
Copy link
Collaborator Author

@LarissaReames-NOAA Is there still work going on in this PR, or is there another reason that it is in draft mode? Also, I'm not seeing an associated ufs-weather-model PR.

I think we're done with all of the ccpp-physics reviews at this point? I can go ahead and move from draft to active. I guess I wasn't sure if I needed a UWM PR if no changes were required in that repo for this PR to work. I've never submitted a PR that didn't have at least a change or two at the UWM level.

@LarissaReames-NOAA LarissaReames-NOAA marked this pull request as ready for review January 10, 2025 20:34
@grantfirl
Copy link
Collaborator

@LarissaReames-NOAA Is there still work going on in this PR, or is there another reason that it is in draft mode? Also, I'm not seeing an associated ufs-weather-model PR.

I think we're done with all of the ccpp-physics reviews at this point? I can go ahead and move from draft to active. I guess I wasn't sure if I needed a UWM PR if no changes were required in that repo for this PR to work. I've never submitted a PR that didn't have at least a change or two at the UWM level.

Yes, ufs-community/ccpp-physics#241 has all the approvals it needs.

It's OK to have a UWM PR with only an update to submodules, fv3atm and ccpp-physics in this case. This happens all the time when there is a physics-only change. UFS code managers still need something to "attach" submodule changes to in order to test it. Plus, when you run regression tests on whatever platform that you have access to, you need to upload your "pre-test" RT log and the test_changes.list file, so the UWM PR is never completely empty even with only submodule changes.

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