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

Diagnostic tendencies ("nophys" variables) are not written out properly #159

Closed
grantfirl opened this issue Aug 17, 2020 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@grantfirl
Copy link
Collaborator

Description

When the ldiag3d and qdiag3d variables are set to T, diagnostic physics tendencies are calculated as well as tendencies saving all non-physics changes to the state variables (with "_nophys" attached to the CCPP standard names). One regression test, named fv3_ccpp_gsd_diag3d_debug, exercises this option. However, the nophys variables are not being written out correctly. There is a mismatch between the module in the diag_table for these variables and what is in GFS_diagnostics.F90.

It has been suggested that the "nophys" variables be part of the "gfs_dyn" module rather than the "gfs_phys" module since they do not correspond to actual physics. However, in the GFS_diagnostics.F90 code, all "nophys" variables (dt3dt_nophys, dq3dt_nophys, du3dt_nophys, dv3dt_nophys, dq3dt_oznophys) are given the "gfs_phys" module. Also, In the diag_table used for this regression test, all nophys variables are assumed to be in the gfs_dyn module, except for dq3dt_nophys.

To Reproduce:

On Hera, when running the fv3_ccpp_gsd_diag3d_debug regression test using the "develop" branch and all of its associated submodules, the output history files (dynf00X.tileY.nc and phyf00X.tileY.nc) do not contain the variables requested to be written out in the diag_table, except for the dq3dt_nophys variable, which was given the wrong module in the diag_table.

Additional context

This functionality is needed for driving the CCPP SCM with FV3 output and for other physics-related studies looking at tendencies.

Output

N/A

Testing:

The code in https://github.com/grantfirl/fv3atm/tree/fix_nophys_tendencies_output together with the attached diag_table (which should go in /scratch1/NCEPDEV/nems/emc.nemspara/RT/NEMSfv3gfs/develop-20200812/INTEL/FV3_input_data_gsd/) was used on Hera/Intel and, as part of the aforementioned regression test, will correctly output all "nophys" variables within the phyf00X.tileY.nc files.
diag_table_gsd_ruc_diag3d.zip

Note that this update will cause the fv3_ccpp_gsd_diag3d_debug regression test to fail because the phyf00X.tileY.nc files will now contain new variables, although it does not "change the answer". All other regression tests are unaffected.

Dependent PRs:

N/A

@grantfirl grantfirl added the bug Something isn't working label Aug 17, 2020
@grantfirl
Copy link
Collaborator Author

See https://github.com/grantfirl/fv3atm/tree/fix_nophys_tendencies_output for the fix with the modified diag_table_gsd_ruc_diag3d staged in the appropriate place by someone with permissions in the /scratch1/NCEPDEV directory.

@grantfirl
Copy link
Collaborator Author

@climbfuji Could the changes in the branch listed above be combined into another PR (since it only changes 5 lines in GFS_diagnostics.F90: develop...grantfirl:fix_nophys_tendencies_output) plus a change in a RT diag_table file? Or should this be a standalone PR?

@climbfuji
Copy link
Collaborator

@climbfuji Could the changes in the branch listed above be combined into another PR (since it only changes 5 lines in GFS_diagnostics.F90: develop...grantfirl:fix_nophys_tendencies_output) plus a change in a RT diag_table file? Or should this be a standalone PR?

I think we can use your PR #162 and make the changes there. At the same time, update the diag tables as per @jkhender's suggestion for the non-ldiag3d/qdiag3d RAP/HRRR physics runs.

@grantfirl
Copy link
Collaborator Author

@climbfuji Could the changes in the branch listed above be combined into another PR (since it only changes 5 lines in GFS_diagnostics.F90: develop...grantfirl:fix_nophys_tendencies_output) plus a change in a RT diag_table file? Or should this be a standalone PR?

I think we can use your PR #162 and make the changes there. At the same time, update the diag tables as per @jkhender's suggestion for the non-ldiag3d/qdiag3d RAP/HRRR physics runs.

OK, this fix has been added to PR #162 : b1ca551

I don't have access to the other diag tables changes from @jkhender, but I'd be happy if the diag_table in the .zip file uploaded above is used in the RTs going forward at some point.

@climbfuji
Copy link
Collaborator

climbfuji commented Oct 1, 2020

This has been fixed in #178, please close the issue.

KevinViner-NOAA pushed a commit to KevinViner-NOAA/fv3atm that referenced this issue Mar 6, 2023
* Add changes from Jun Wang for fast read restart files

* Add changes from Tanya.

* latest bugfix to fv_regional_bc.F90

* update submodule ccpp

* merge rrfs_baseE into ccpp-physics and GFDL_atmos_cubed_sphere

* remove FV3GFS_io_netcdf

* remove FV3GFS_io_netcdf

* remove two absent files from CMakeLists.txt

* remove two absent files from CMakeLists.txt

Co-authored-by: Ming.Hu <Ming.Hu@noaa.gov>
LarissaReames-NOAA pushed a commit to LarissaReames-NOAA/fv3atm that referenced this issue Nov 17, 2023
LarissaReames-NOAA pushed a commit to LarissaReames-NOAA/fv3atm that referenced this issue Nov 17, 2023
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

No branches or pull requests

2 participants