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

Bug fixes to 3D diagnostic tendencies; add more tendencies #18

Merged

Conversation

SamuelTrahanNOAA
Copy link
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA commented Mar 6, 2020

Bug fixes are added so the 3D diagnostic tendency output works properly. There is a new qdiag3d flag to turn on 3D tracer tendencies. Also, there are now tendencies for the model, ccpp, and total. The later two are for debugging the tendency calculations. The timestep_init and timestep_final stages of CCPP are implemented.

This is fully tested with gfs v16 beta, GSD v0, and GFS v15p2 suites.

However, there is an unresolved question of how to handle the problem of precision.

The Precision Problem

I can say definitively that double precision floating point is not enough to accumulate tendencies across the orders of magnitude typically found in the atmosphere. This was determined by adding a fourth tendency, now disabled in the code. Every time the ccpp or model tendencies were accumulated, that fourth tendency was accumulated by the same amount. In that case, the tendencies of: temperature had a precision of about 0.1 degrees, water vapor mixing ratio of 1e-5, and wind of .1 meters per second. Those are per-3-hour tendencies from a C768 global model. A higher-resolution run would have much larger imprecision due to the larger number of timesteps.

Those can be viewed as the best possible tendency errors.

Right now, the error in the tendencies of CCPP are about 2.5 times the best possible. It is hard to tell whether it is due to further precision issues (eight times as many additions per timestep) or incorrect tendency logic.

The precision issue could be dealt with using quadruple precision (128 bit) floating-point. That is a part of the Fortran 2008 standard, and it is an optional part of the MPI standard. It may be feasible to implement it, but that could cause portability issues.

@DomHeinzeller
Copy link

@llpcarson @grantfirl - for your information, the latest version of the 3D tendencies PR for NOAA-GSD/ccpp-physics.

@@ -0,0 +1,87 @@
!>\file model_tend_post.F90
!! Calculates tendencies from all processes outside of CPPP

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using "ccpp" here seems like the wrong terminology, or at least, it's not clear to me. Is this "outside of physics", i.e. dycore? Or, ?

Copy link

@DomHeinzeller DomHeinzeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, apologies that it took me so long to review these PRs. I do have a few concerns about the implementation, which I also ran past the developers of the next-generation CCPP framework code generator - to make sure that I am not giving wrong directions wrt future development. The following is based on my understanding of what the CCPP is required/expected to do, and I may have missed important details.

The issue I am seeing is that code is added to the ccpp-physics repository that is responsible for calculating model tendencies before/after the actual physics are run. This is not the responsibility of the CCPP. The CCPP is "subordinate" to the host model and does what it is supposed to do, run the physics, return updated quantities and tendencies of so requested. It is the host model's responsibility to calculate its own tendencies from the dycore and other processes, and use the tendencies from CCPP for output or whatever else it wants to do.

As such, I believe that the code in physics/model*.F90 does not belong inside the CCPP. This code belongs to the host model and could be coded up in atmos_model.F90 (or maybe CCPP_driver.F90, but I will need to think about this a little more). This also means that the newly added timestep_init and timestep_final groups are not necessary, and some of the variables added to GFS_typedefs.F90 (e.g. the bookkeeping array) may not be required either.

Regarding the changes to the actual physics schemes, I think this is really good. The idea to use flags to signal of developers have already put the code to populate the 3d physics tendency arrays inside their schemes or not is much better than "if gf then ... else if sas then ... end if". For the moment this is hard-coded in GFS_typedefs.F90, which is ok - the current CCPP framework does not provide any other way. With cap_gen.py, we may be able to obtain this information from the CCPP itself (metadata, suite definition file) and remove the hard-coded section in GFS_typedefs.F90. Again, much better than having to weed through the physics schemes and take out if blocks based on which scheme is run.

I would like to get your feedback (in particular Sam) and, if what I am envisioning is ok, permission to propose changes on top of these PRs so that all the good work that went into the existing physics schemes gets used.

@llpcarson @grantfirl @ligiabernardet @gold2718 @SamuelTrahanNOAA

@SamuelTrahanNOAA
Copy link
Collaborator Author

I added the model, ccpp, and total tendencies for my own purposes, to debug the tendency calculations. I don't see their addition to the master as being critical. It is quite possible it isn't useful to anyone but me.

I suggest they be entirely removed from this PR. If they're deemed needed in the future, I can make a new PR.

@DomHeinzeller
Copy link

I added the model, ccpp, and total tendencies for my own purposes, to debug the tendency calculations. I don't see their addition to the master as being critical. It is quite possible it isn't useful to anyone but me.

I suggest they be entirely removed from this PR. If they're deemed needed in the future, I can make a new PR.

Ok, I will work on this while you are busy with the RAP implementation changes, and ask you to review the combined/amended PRs once they are ready. Thanks!

@DomHeinzeller DomHeinzeller added the do not merge Something is wrong, do not merge label Apr 20, 2020
DomHeinzeller added a commit that referenced this pull request Apr 22, 2020
Bug fixes to 3D diagnostic tendencies (based on #18)
@DomHeinzeller DomHeinzeller merged commit e4b80ea into NOAA-GSL:gsd/develop Apr 22, 2020
mdtoyNOAA pushed a commit to mdtoyNOAA/ccpp-physics that referenced this pull request Mar 29, 2022
SamuelTrahanNOAA pushed a commit that referenced this pull request Jun 14, 2022
guoqing-noaa added a commit to guoqing-noaa/ccpp-physics that referenced this pull request Feb 22, 2023
…d)+cherry-pick (5544dab - 42184e7)

commit 77bcfb1
Merge: f13ed4e 2d2f1a6
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Fri Jun 3 09:35:42 2022 -0400

    Merge pull request NCAR#903 from lisa-bengtsson/prog_closure

    Add prognostic cumulus closure description in saSAS

commit 2d2f1a6
Merge: 48f4274 f13ed4e
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Fri May 27 13:53:46 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit f13ed4e
Merge: 6e58242 942f9ad
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Thu May 26 16:34:40 2022 -0400

    Merge pull request NCAR#918 from SamuelTrahanNOAA/ccpp-neptune

    NRL Neptune model 32-bit physics support

commit 48f4274
Merge: fc79cc3 6e58242
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu May 26 15:05:43 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit 942f9ad
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Wed May 25 22:08:44 2022 +0000

    correct bug in machine.F

commit 641544c
Merge: d4d0b71 6e58242
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Wed May 25 18:40:09 2022 +0000

    Merge remote-tracking branch 'community/main' into ccpp-neptune

commit 6e58242
Merge: 01e3d6b 8dae03a
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Wed May 25 13:54:59 2022 -0400

    Merge pull request NCAR#924 from dustinswales/update_rte_for_CCPP_v6

    Update rte-rrtmgp submodule + SCM-only bugfix

commit d4d0b71
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Tue May 24 18:18:45 2022 +0000

    Simplify machine.F and remove unused types.

commit 828f168
Merge: 63020ec 01e3d6b
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Mon May 23 18:50:11 2022 +0000

    Merge NCAR main

commit 8dae03a
Merge: b994063 87359d2
Author: dustinswales <dswales@ucar.edu>
Date:   Fri May 20 09:46:35 2022 -0600

    Merge pull request NOAA-GSL#18 from grantfirl/fix_SCM_specified_surface_flux_bug

    Fix scm specified surface flux bug

commit 87359d2
Merge: 49c7096 01e3d6b
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Fri May 20 09:13:55 2022 -0400

    Merge branch 'main' into fix_SCM_specified_surface_flux_bug

commit fc79cc3
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu May 19 19:59:43 2022 +0000

    Change intent to inout for conditional variables

commit 96d0d36
Merge: 6f38cc6 01e3d6b
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu May 19 00:14:56 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit 6f38cc6
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Wed May 18 23:58:09 2022 +0000

    address some review comments, fix decomposition error, correct bug in initialization

commit b994063
Author: Dustin Swales <dustin.swales@noaa.gov>
Date:   Mon May 16 09:16:30 2022 -0600

    Update rte-rrtmgp submodule

commit 49c7096
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Tue May 10 13:49:55 2022 -0400

    make sure that tsfc_wat is calculated when wet = T

commit 63020ec
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Thu May 5 22:46:10 2022 +0000

    Switch to another version of the code that works with 64 bit

commit 3dec4e6
Merge: be534e7 3405ff1
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu May 5 04:20:02 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit be534e7
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu May 5 04:08:06 2022 +0000

    addressing some review comments

commit e7c42c7
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Thu May 5 00:43:21 2022 +0000

    Move some code to modules

commit de90593
Merge: 6871a93 3405ff1
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Wed May 4 22:12:14 2022 +0000

    Merge remote-tracking branch 'community/main' into sing_prec_from_main

commit 6871a93
Author: Samuel Trahan <Samuel.Trahan@noaa.gov>
Date:   Wed May 4 17:32:24 2022 +0000

    Changes needed for 32-bit physics

commit 527e1b9
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Mon May 2 22:11:42 2022 +0000

    Pass -DCCPP_SINGLE_PRECISION from cmake to -DSINGLE_PREC in cpp

commit aff574b
Merge: 239710b 7e35351
Author: samuel.trahan <Samuel.Trahan@noaa.gov>
Date:   Mon May 2 16:02:18 2022 +0000

    Merge remote-tracking branch 'community/main' into sing_prec_from_main

commit 8b815e0
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Fri Apr 29 03:03:46 2022 +0000

    address some bugs caught by debug flag

commit e2d5a2a
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Wed Apr 27 22:40:40 2022 +0000

    cleaning out some print statements

commit 0200e2d
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Wed Apr 27 18:48:38 2022 +0000

    addressing some review comments

commit e969672
Merge: a9b439f 7e35351
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Fri Apr 22 21:10:40 2022 +0000

    merge with upstream

commit a9b439f
Merge: fc7e7a0 860245c
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Fri Apr 22 04:07:35 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit fc7e7a0
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Fri Apr 22 04:00:31 2022 +0000

    addressing some review comments

commit 89eaad9
Merge: b530db1 e2806f0
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Thu Apr 21 15:05:15 2022 +0000

    Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure

commit b530db1
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Wed Apr 20 01:29:13 2022 +0000

    cleaning some diagnostics

commit 4f84ed7
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Tue Apr 19 19:22:53 2022 +0000

    add shallow convection closure updates, add ntsigma in generic files

commit 842eae3
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Mon Apr 18 15:05:40 2022 +0000

    Ensuring the moisture budget is correct via PBL, microphysics coupling

commit 235ec38
Author: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>
Date:   Wed Apr 13 17:44:27 2022 +0000

    add progsigma_calc

commit 239710b
Merge: b17a7e7 8500cea
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Thu Jan 13 11:06:52 2022 -0700

    Merge branch 'main' into sing_prec_from_main

commit b17a7e7
Merge: 805c62c 623feaa
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Tue Dec 7 11:00:17 2021 -0700

    Merge branch 'main' into sing_prec_from_main

commit 805c62c
Author: Grant Firl <grant.firl@noaa.gov>
Date:   Tue Dec 7 10:39:05 2021 -0700

    add single precision code changes from michalakes fork, jm-nrl-32bitfp-24cc09e branch
christinaholtNOAA pushed a commit that referenced this pull request Mar 2, 2023
Update to parametric values in prognostic convection for GFSv17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Something is wrong, do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants