-
Notifications
You must be signed in to change notification settings - Fork 118
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
User/wfc/climate nudging #47
Conversation
These diagnostics need to be output for high time frequency (4xdaily) so outputting the full PV field (>=33 levels) would consume too much disk space for longer runs.
Default values should reproduce previous (non-ECDA) answers For ECDA add the following to fv_nwp_nudge_nml p_relax=10.e2 using_merra2 = .true. climate_nudging=.true.
Moved climate_nudging from private to shared declaration.
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.
You made changes to fv_nudge.F90, which has historically been used for weather timescales by the Weather and Climate Dynamics Division at GFDL. Tests will need to be run with this version to ensure it does not change answers when used in for their purposes.
Hi, Rusty. Thanks. Perhaps one of the regression tests should be a nudging
test? It would not have to be particularly high-resolution, c48 or c192
would be fine.
Lucas
…On Fri, Aug 28, 2020 at 11:07 AM bensonr ***@***.***> wrote:
***@***.**** commented on this pull request.
You made changes to fv_nudge.F90, which has historically been used for
weather timescales by the Weather and Climate Dynamics Division at GFDL.
Tests will need to be run with this version to ensure it does not change
answers when used in for their purposes.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#47 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVEXV7CDRRQFQR6D6QDSC7B37ANCNFSM4QNHS4UQ>
.
|
@lharris4 I think that's a good idea from a code coverage perspective.
Once we've used the test to assure this PR is safe, let's work to get this
in the proper RTS system we are putting in place.
|
This change has been tested in SHiELD and the results are consistent. |
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.
I believe this is still crashing when running with SPEAR. There is a variable that needs to be initialized. This is the traceback for the crash that I get:
fms_SPEAR_Q2020.0 0000000003238C73 mpp_mod_mp_mpp_er 68 mpp_util_mpi.inc
fms_SPEAR_Q2020.0 00000000030E1DE0 fms_io_utils_mod_ 164 fms_io_utils.F90
fms_SPEAR_Q2020.0 000000000300D4BB netcdf_io_mod_mp_ 284 netcdf_io.F90
fms_SPEAR_Q2020.0 0000000002FD795A fms_netcdf_domain 921 domain_write.inc
fms_SPEAR_Q2020.0 00000000030C896A diag_output_mod_m 1265 diag_output.F90
fms_SPEAR_Q2020.0 0000000002E01481 diag_util_mod_mp_ 2650 diag_util.F90
fms_SPEAR_Q2020.0 0000000002FB5100 diag_manager_mod_ 3544 diag_manager.F90
fms_SPEAR_Q2020.0 0000000002FA1264 diag_manager_mod_ 2034 diag_manager.F90
fms_SPEAR_Q2020.0 0000000002F90A47 diag_manager_mod_ 1579 diag_manager.F90
fms_SPEAR_Q2020.0 00000000005DCA76 fv_diagnostics_mo 2082 fv_diagnostics.F90
fms_SPEAR_Q2020.0 000000000049A9E9 atmosphere_mod_mp 957 atmosphere.F90
fms_SPEAR_Q2020.0 000000000047A0CA atmos_model_mod_m 1068 atmos_model.F90
fms_SPEAR_Q2020.0 0000000000403731 MAIN__ 970 coupler_main.F90
@uramirez8707 do you have a fix for this crash? |
This is all that is needed: The crash happens for File= This does not crash when using FMS-2019.01.02 + GFDL_atmos_cubed_sphere - 2020.01 One of the changes between GFDL_atmos_cubed_sphere 2020.01 and 2020.02 is the removal of the GFDL_atmos_cubed_sphere/tools/fv_diagnostics.F90 Line 1557 in 72bbfcc
GFDL_atmos_cubed_sphere/tools/fv_diagnostics.F90 Line 2047 in 2aac011
The Initiliazing |
@lharris4 @bensonr Is there a reason that lines 3072-3076 from the GFDL_atmos_cubed_sphere/tools/fv_diagnostics.F90 Lines 3072 to 3076 in 19b48bc
|
@laurenchilutti I don't think we want to reinstate computation for pressures that may be below the surface of the earth. I agree the height should be either missing value or 0. |
@wfcooke Please see the comment from @uramirez8707 and @thomas-robinson regarding crashes due to bad values for a3 from the get_height_given_pressure computation. As @laurenchilutti points out, the calculation may abort early in high altitude areas. Decide if your calculation should produce zeros or missing-value for those areas or missing_value and update the code accordingly. Once you have done this, the updates can be merged. |
Rusty, Uriel, Tom
I talked to the folks in my division about this update and they think a
missing value is probably the best way to go.
The segment of code you pointed to was not a place that I checked in a
modiffication for.
If I read the "blame" field properly it seems to be related to EMC (checked
in by Rusty?). I don't want to mess them up.
Can you guys change that part of the code or do you need me to add a
a2(i,j,n) =missing_value
before the 1000 continue line?
Will
…On Fri, Sep 18, 2020 at 2:07 PM bensonr ***@***.***> wrote:
@wfcooke <https://github.com/wfcooke> Please see the comment from
@uramirez8707 <https://github.com/uramirez8707> and @thomas-robinson
<https://github.com/thomas-robinson> regarding crashes due to bad values
for a3 from the get_height_given_pressure computation. As @laurenchilutti
<https://github.com/laurenchilutti> points out, the calculation may abort
early in high altitude areas. Decide if your calculation should produce
zeros or missing-value for those areas or missing_value and update the code
accordingly. Once you have done this, the updates can be merged.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHCVICQLOAEKKZQAG73PPDSGOLKJANCNFSM4QNHS4UQ>
.
|
@wfcooke the removal was not done by or for EMC. Please make the update
you suggested so we can get this merged into the dev/gfdl branch.
|
…alize to model/fv_dynamics.F90 (NOAA-GFDL#47) Implementation of CCPP timestep_init and timestep_final phases in model/fv_dynamics.F90 Note that while these new phases are currently not doing any work, they are required for transitioning to the new CCPP code generator capgen.py (scheduled for February 2021), at which time they will be taking over some of the work that is currently done manually (allocating data for the CCPP fast physics calls).
Merge Geos/develop into geos/main
Updates for PV350K, PV550K diagnostics and some climate nudging updates our division needs.