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

FHZERO bug #908

Closed
pjpegion opened this issue Nov 10, 2021 · 10 comments · Fixed by #918
Closed

FHZERO bug #908

pjpegion opened this issue Nov 10, 2021 · 10 comments · Fixed by #918
Labels
bug Something isn't working

Comments

@pjpegion
Copy link
Collaborator

Description

Changing the output frequency affect the resulting forecast.

To Reproduce:

What compilers/machines are you seeing this with?
Intel compiler in hera

Give explicit steps to reproduce the behavior.

  1. Checkout latest ufs-weather-model
  2. copy the control_p7 test to control_p7_fhzero
  3. add export FHZERO=3 to control_p7_fhzero
  4. run both control_p7 and control_p7_fhzero tests keeping the output

Compare the restart files, and any instantaneous field, such as ugrd, and you will see differences

Additional context

I tested this with "control", which is GFS_V16 physics, and that physics suite doesn't contain this bug.

@pjpegion pjpegion added the bug Something isn't working label Nov 10, 2021
@pjpegion
Copy link
Collaborator Author

I did some backing off of p7, and when I turn off the GWD the problem goes away.

@junwang-noaa
Copy link
Collaborator

Thanks Phil for identifying the feature that causes the problem. I did some test runs with control_p7 and confirmed that the results are changed when the value of fhzero is changed. @yangfanglin @mdtoy FYI.

@yangfanglin
Copy link
Collaborator

Is using FHZERO=6 working fine with uGWD ?

@junwang-noaa
Copy link
Collaborator

The instant fields should not be changed if a different FHZERO is used. Since they do, I guess there might be some issue with FHZERO related computation with uGWD

@mdtoy
Copy link

mdtoy commented Nov 10, 2021 via email

@pjpegion
Copy link
Collaborator Author

pjpegion commented Nov 10, 2021

@mdtoy I found the bug. The precipitation going into ugwp is from the Diagnostic container, and should be coming from the surface container. It is just a few lines switches in the meta files:

diff --git a/physics/ugwpv1_gsldrag.meta b/physics/ugwpv1_gsldrag.meta
index 64d6b0d6..86b46477 100644
--- a/physics/ugwpv1_gsldrag.meta
+++ b/physics/ugwpv1_gsldrag.meta
@@ -595,8 +595,8 @@
   kind = kind_phys
   intent = in
 [rain]
-  standard_name = lwe_thickness_of_precipitation_amount_on_dynamics_timestep
-  long_name = total rain at this time step
+  standard_name = nonnegative_lwe_thickness_of_precipitation_amount_on_dynamics_timestep
+  long_name = total precipitation amount in each time step
   units = m
   dimensions = (horizontal_loop_extent)
   type = real
diff --git a/physics/unified_ugwp.meta b/physics/unified_ugwp.meta
index 54725668..14179862 100644
--- a/physics/unified_ugwp.meta
+++ b/physics/unified_ugwp.meta
@@ -1014,8 +1014,8 @@
   kind = kind_phys
   intent = in
 [rain]
-  standard_name = lwe_thickness_of_precipitation_amount_on_dynamics_timestep
-  long_name = total rain at this time step
+  standard_name = nonnegative_lwe_thickness_of_precipitation_amount_on_dynamics_timestep
+  long_name = total precipitation amount in each time step
   units = m
   dimensions = (horizontal_loop_extent)
   type = real

@junwang-noaa
Copy link
Collaborator

Excellent! Thanks for fixing the bug, Phil!

@climbfuji
Copy link
Collaborator

Yes, thanks very much Phil, great detective work.

@mdtoy
Copy link

mdtoy commented Nov 16, 2021

@pjpegion Thank you again for finding the bug. I implemented your fix to the meta data files and verified that it indeed fixes the forecast reproducibility issue associated with fhzero. That was a tough bug, and I learned something from what you found.
@climbfuji This looks like a problem that could pop up again from time to time, and that could already exist in other physics schemes. The "fhzero reproducibility" test should probably be added to the suite of Regression Tests.
@climbfuji Do you want me to issue a PR for this bug? Or is it already being handled?

@climbfuji
Copy link
Collaborator

@pjpegion Thank you again for finding the bug. I implemented your fix to the meta data files and verified that it indeed fixes the forecast reproducibility issue associated with fhzero. That was a tough bug, and I learned something from what you found.
@climbfuji This looks like a problem that could pop up again from time to time, and that could already exist in other physics schemes. The "fhzero reproducibility" test should probably be added to the suite of Regression Tests.

That's a good point, maybe as part of the OpnReqTest - @MinsukJi-NOAA @junwang-noaa what do you think?

@climbfuji Do you want me to issue a PR for this bug? Or is it already being handled?

Yes, please create a PR and we will include it as soon as possible. I could, for example, pull your new PR into NCAR/ccpp-physics#775, if it doesn't change the regression test results. Please state if it does or not in your PR. Thank you @mdtoy and @pjpegion .

mdtoy pushed a commit to mdtoy/ufs-weather-model that referenced this issue Nov 16, 2021
mdtoy pushed a commit to mdtoy/ufs-weather-model that referenced this issue Nov 17, 2021
…adata files to fix ufs-community/ufs-weather-model 'fhzero reproducibility' issue ufs-community#908
@DeniseWorthen DeniseWorthen linked a pull request Nov 17, 2021 that will close this issue
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants