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

Implement CCPPized check_energy_chng and check_energy_fix #145

Merged
merged 42 commits into from
Nov 18, 2024

Conversation

jimmielin
Copy link
Member

@jimmielin jimmielin commented Oct 25, 2024

Fixes #114

This PR fixes the following NCAR/atmospheric_physics Github issues: #114

  • Implements check_energy_chng. The routine computes total energies using physics and dycore formula and takes in boundary fluxes of vapor, liquid+ice, ice, sensible heat, and writes to log significant energy conservation errors.
    In order to take in the scheme name and fluxes from the last calling physics scheme (usually zero) a check_energy_zero_fluxes has to be ran before the scheme, then the physics scheme to be checked, then check_energy_chng.

  • Implements check_energy_fix. The routine computes the heating rate required for global mean total energy conservation which is later applied by apply_heating_rate.
    Supporting routines include the global mean calculator for total energy (check_energy_gmean) - stored in separate folder to prevent CAM from building it.
    The global means are very useful for diagnosing model state (as the computed energy numbers include all model state incl. constituents) through a simple one-line printout. check_energy_gmean_diagnostics implements this.
    At the end of the timestep check_energy_save_teout has to be ran in order to save total energies at the end of the timestep for use in the next timestep.

  • Diagnostics of intermediate quantities used in check_energy_diagnostics.

List all existing files that have been added (A), modified (M), or deleted (D),
and describe the changes:

- Docs:
M       doc/ChangeLog

- check_energy_chng and supporting routines:
A       schemes/check_energy/check_energy_chng.F90
A       schemes/check_energy/check_energy_chng.meta
A       schemes/check_energy/check_energy_chng_namelist.xml
A       schemes/check_energy/check_energy_scaling.F90
A       schemes/check_energy/check_energy_scaling.meta
A       schemes/check_energy/check_energy_zero_fluxes.F90
A       schemes/check_energy/check_energy_zero_fluxes.meta

- check_energy_fix and supporting routines:
A       schemes/check_energy/check_energy_fix.F90
A       schemes/check_energy/check_energy_fix.meta
A       schemes/check_energy/check_energy_gmean/check_energy_gmean.F90
A       schemes/check_energy/check_energy_gmean/check_energy_gmean.meta
A       schemes/check_energy/check_energy_save_teout.F90
A       schemes/check_energy/check_energy_save_teout.meta

- check_energy related diagnostics:
A       schemes/sima_diagnostics/check_energy_diagnostics.F90
A       schemes/sima_diagnostics/check_energy_diagnostics.meta

- check_energy_gmean "nstep, te" atm.log output:
A       schemes/sima_diagnostics/check_energy_gmean_diagnostics.F90
A       schemes/sima_diagnostics/check_energy_gmean_diagnostics.meta

- test SDF including all functionality:
A       test/test_suites/suite_check_energy.xml

List and Describe any test failures: N/A

Summarize any changes to answers: none

jimmielin and others added 25 commits October 25, 2024 11:45
…s); namelist variable control for print_energy_errors
@nusbaume nusbaume self-requested a review October 28, 2024 15:35
@jimmielin
Copy link
Member Author

Hi @nusbaume, I've added adiabatic scheme and tested with --analytic-ic and snapshots, and added dycore_energy_consistency_adjust CCPP-ized scheme. Ready for review again now. Thank you!

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Great work @jimmielin! I realize it appears that I have a bunch of requests, but hopefully most of them are fairly straight-forward to implement, or can be pushed off to a future PR. I also tried to only make a suggestion for a given standard name in a single comment, but could have certainly repeated myself.

Also, some of my standard name suggestions are different than what was in the spreadsheet. This is mostly motivated by what the standard name usage actually looks like now that we have an example, and by recent standard name rule discussions we have had with NOAA and others. Of course I am happy to receive push back on it if you or the scientists disagree.

Thanks again!

schemes/check_energy/check_energy_chng_namelist.xml Outdated Show resolved Hide resolved
suites/suite_adiabatic.xml Outdated Show resolved Hide resolved
suites/suite_adiabatic.xml Show resolved Hide resolved
suites/suite_cam7.xml Show resolved Hide resolved
suites/suite_adiabatic.xml Show resolved Hide resolved
schemes/check_energy/check_energy_chng.meta Outdated Show resolved Hide resolved
schemes/check_energy/check_energy_chng.meta Outdated Show resolved Hide resolved
schemes/check_energy/check_energy_chng.meta Outdated Show resolved Hide resolved
schemes/check_energy/check_energy_chng.meta Outdated Show resolved Hide resolved
schemes/check_energy/check_energy_chng.meta Outdated Show resolved Hide resolved
character(len=64), intent(out) :: name ! parameterization name for fluxes
real(kind_phys), intent(out) :: flx_vap(:) ! boundary flux of vapor [kg m-2 s-1]
real(kind_phys), intent(out) :: flx_cnd(:) ! boundary flux of liquid+ice (precip?) [m s-1]
real(kind_phys), intent(out) :: flx_ice(:) ! boundary flux of ice (snow?) [m s-1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove "(snow?)"

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed!

suites/suite_adiabatic.xml Show resolved Hide resolved
suites/suite_held_suarez_1994.xml Outdated Show resolved Hide resolved
schemes/sima_diagnostics/check_energy_diagnostics.F90 Outdated Show resolved Hide resolved
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Looks like Github already notified of you of my re-review comments (?), so I will submit this now to see if I can get Github to reset on my end (apologies if this causes any confusion).

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for resolving most of my concerns @jimmielin! I did have a few unresolved requests, but once those are resolved then I think this PR will be good to go. Of course please let me know if something is unresolved but you are unsure as to why (as I may have just missed it). Thanks!

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Everything looks great to me now. Thanks again @jimmielin!

@jimmielin
Copy link
Member Author

Regression tests using the latest commit in ESCOMP/CAM have passed (with expected existing failures):

aux_cam_nag_20241118102125: 39 tests
  DAE.f45_f45_mg37.FHS94.izumi_nag.cam-dae (Overall: PEND) details:
    PEND DAE.f45_f45_mg37.FHS94.izumi_nag.cam-dae RUN
    PEND DAE.f45_f45_mg37.FHS94.izumi_nag.cam-dae COMPARE_base_da
  ERC_D_Ln9.f10_f10_mg37.QPC5.izumi_nag.cam-outfrq3s_subcol (Overall: PEND) details:
    PEND ERC_D_Ln9.f10_f10_mg37.QPC5.izumi_nag.cam-outfrq3s_subcol RUN
    PEND ERC_D_Ln9.f10_f10_mg37.QPC5.izumi_nag.cam-outfrq3s_subcol COMPARE_base_rest

aux_cam_gnu_20241118103702: 29 tests

aux_cam_intel_20241118102506: 58 tests
  ERP_Ln9.f09_f09_mg17.FCSD_HCO.derecho_intel.cam-outfrq9s (Overall: FAIL) details:
    FAIL ERP_Ln9.f09_f09_mg17.FCSD_HCO.derecho_intel.cam-outfrq9s COMPARE_base_rest
  SMS_D_Ln9.f19_f19_mg17.FXHIST.derecho_intel.cam-outfrq9s_amie (Overall: FAIL) details:
    FAIL SMS_D_Ln9.f19_f19_mg17.FXHIST.derecho_intel.cam-outfrq9s_amie SETUP
  SMS_D_Ln9_P1280x1.ne0CONUSne30x8_ne0CONUSne30x8_mt12.FCHIST.derecho_intel.cam-outfrq9s (Overall: FAIL) details:
    FAIL SMS_D_Ln9_P1280x1.ne0CONUSne30x8_ne0CONUSne30x8_mt12.FCHIST.derecho_intel.cam-outfrq9s SETUP

aux_cam_nvhpc_20241118111853: 1 test

Filling in ChangeLog tag atmos_phys0_07_000 then merging. Thanks everyone for the reviews and patience!

@jimmielin jimmielin merged commit 79df94a into ESCOMP:development Nov 18, 2024
3 checks passed
jimmielin added a commit that referenced this pull request Nov 19, 2024
Tag name (The PR title should also include the tag name):
`atmos_phys0_07_000`
Originator(s): @jimmielin

List all `development` PR URLs included in this PR and a short
description of each:
* #141 by @mattldawson @boulderdaze 
* #147 by @peverwhee 
* #144 by @mwaxmonsky 
* #151 by @mattldawson 
* #145 by @jimmielin 

List all test failures: N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tag
Development

Successfully merging this pull request may close these issues.

3 participants