-
Notifications
You must be signed in to change notification settings - Fork 153
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
UGWP scheme is using its own set of constants rather than the host-provided ones #304
Comments
See #298 for details. Further issues that need to be addressed are print statements. CCPP actually doesn't allow schemes to write to stdout, but in practice this has not been followed in the absence of a proper logging system. We should at least revisit them, clean them up and/or acticate them only in debug mode. Further, a goto statement in one of the modules needs to be replaced with proper, modern Fortran syntax |
Both the IPD- and the CCPP-version UGWP have the above mentioned "constant, stdout, and goto" issues, because the relevant subroutines and modules in ccpp/physics/physics are exactly the same as those in the IPD's physics. A fix is needed ASAP before the issue get forgotten and move down in the issue list. A new baseline will need to be established once the constants are all coming from FV3 properly. A b4b is required after the fix. |
@grantfirl Do you want to keep this open as long as the UGWP v0 is around? Can you confirm that this has been resolved to your satisfaction for UGWP v1? |
I don't know. I think that this issue was originally started for the UGWP v0 scheme, which is still using its own constants, so in that sense, the issue is unaddressed. However, the UGWP v1 scheme does look like they followed the guidance and are setting the scheme-internal constants module values from the CCPP-provided ones (see ugwpv1_gsldrag_init in https://github.com/NCAR/ccpp-physics/blob/master/physics/ugwpv1_gsldrag.F90), so the physical constants are being controlled by host and are consistent with other schemes now in that version. I guess what to do about this issue depends on UGWPv0's future. If UGWPv1 completely replaces UGWPv0, then we can close this issue with a wontfix label. If UGWPv0 is to stay alongside UGWPv1 for the forseeable future, then we can leave the issue alone until it is fixed (by ?). |
Grant,
Yes, the plan is to use uGWD.v1 to replace uGWD.v0. Testing and
evaluation are underway.
Fanglin
…On Thu, Apr 1, 2021 at 3:19 PM grantfirl ***@***.***> wrote:
I don't know. I think that this issue was originally started for the UGWP
v0 scheme, which is still using its own constants, so in that sense, the
issue is unaddressed. However, the UGWP v1 scheme does look like they
followed the guidance and are setting the scheme-internal constants module
values from the CCPP-provided ones (see ugwpv1_gsldrag_init in
https://github.com/NCAR/ccpp-physics/blob/master/physics/ugwpv1_gsldrag.F90),
so the physical constants are being controlled by host and are consistent
with other schemes now in that version. I guess what to do about this issue
depends on UGWPv0's future. If UGWPv1 completely replaces UGWPv0, then we
can close this issue with a wontfix label. If UGWPv0 is to stay alongside
UGWPv1 for the forseeable future, then we can leave the issue alone until
it is fixed (by ?).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#304 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKY5N2NR4TDBIJHBIAXE3PTTGTBMBANCNFSM4IPDCPDQ>
.
--
*Fanglin Yang, Ph.D.*
*Chief, Model Physics Group*
*Modeling and Data Assimilation Branch*
*NOAA/NWS/NCEP Environmental Modeling Center*
*https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/
<https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/>*
|
UGWP v0 will be replaced by UGWP v1 in the near future. |
UGWP scheme is not using the host-provided constants but its own ones, which needs to be addressed in a follow-up PR (i.e. constants have to be passed in via cires_ugwp_{init,run,finalize} and then down to this routine.
The text was updated successfully, but these errors were encountered: