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

Restructure ozone code #1276

Merged
merged 7 commits into from
Feb 23, 2021
Merged

Restructure ozone code #1276

merged 7 commits into from
Feb 23, 2021

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Feb 6, 2021

Description of changes

Restructure ozone code to better accommodate the new ozone parameterization in #1232 . The changes here were heavily inspired by @ziu1986 's changes in #1232. My main goal was to demonstrate how we can bring in a new ozone stress formulation using a select case statement rather than object-oriented polymorphism, because this will keep the code simpler. A significant side benefit of this restructure was to allow me to remove the ozone stress terms from the restart file.

Specific notes

Contributors other than yourself, if any: Heavily motivated by changes from @ziu1986 as well as conversations with @sunnivin

CTSM Issues Fixed (include github issue #): none

Are answers expected to change (and if so in what way)? no

Any User Interface Changes (namelist or namelist defaults changes)? no

Testing performed, if any:

ERP_D_Ld5_P48x1.f10_f10_musgs.I2000Clm50Sp.izumi_nag.clm-o3
ERP_D_P36x2_Ld3.f10_f10_musgs.I1850Clm50BgcCrop.cheyenne_intel.clm-default
ERP_P72x2_Ly3.f10_f10_musgs.I2000Clm50BgcCrop.cheyenne_intel.clm-irrig_o3_reduceOutput

All pass and are bit-for-bit with baselines.

I feel like this is cleaner, and I've had problems with sourced
allocation in some cases
This adds up in terms of performance
This will support introducing alternative methods for ozone stress, as
well as removing some variables from the restart file.
This way, we don't need the stress terms on the restart file.
@billsacks billsacks added the PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete label Feb 6, 2021
@billsacks billsacks self-assigned this Feb 6, 2021

select case (this%stress_method)
case (stress_method_lombardozzi)
call this%CalcOzoneStressLombardozzi(bounds, num_exposedvegp, filter_exposedvegp)
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is where this will be extended to allow the Luna-specific method from @ziu1986. I envision that CalcOzoneStress will be called from the same place in the code in either case (currently clm_driver.F90) - i.e., the caller won't need to know which ozone stress method is being used.

It looks like the Luna-based method only needs to calculate the stress coefficient once a day, at the end of the day. So, to give a small performance improvement, we can put an end-of-day check either in the case block (as a conditional around the call to the new routine) or at the top of the new routine (simply returning if it isn't the end of the current day).

In terms of the calculation of the different stress coefficients (of which there will be 6): My thinking is that we can rely on the fact that they are all initialized to 1 in InitCold. So the new o3coefjmax will not be set explicitly in CalcOzoneStressLombardozzi, and similarly, the original o3coefv and o3coefg variables will not be set explicitly in your new method.

Copy link

Choose a reason for hiding this comment

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

Am I right to assume that instead of storing the O3 coefficients we are going to store the O3 uptake in the history/restart files? Not sure if I have seen that in the proposed changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

The o3 uptake was already in the restart files. This is needed because it is an evolving state variable. Previously, we also stored the o3 coefficients in the restart file. That was only needed because of the driver loop ordering: the coefficients were calculated after photosynthesis in time step N and then applied in photosynthesis in time step N+1. I have reordered the code here so that the coefficients are calculated before photosynthesis in time step N, and then applied later that same time step, so they no longer need to be on the restart file.

@billsacks billsacks merged commit 40ad51c into ESCOMP:master Feb 23, 2021
billsacks added a commit that referenced this pull request Feb 23, 2021
Refactor ozone code, and misc. small fixes

(1) Restructure ozone code (#1276) in
    preparation for new ozone parameterization.

(2) Fix non-standard hexadecimal constant
    (#1271), needed for gfortran 10

(3) Remove support for CISM1 (#1226)

(4) Move final WaterGridcellBalance call out to clm_driver (resolves
    #1286)

(5) Only add WA and QCHARGE history fields if use_aquifer_layer is true
    (resolves #1281)

(6) Consolidate conditional structures for VIC initialization (resolves
    #1287)

- Resolves #1286 (Move call to WaterGridcellBalance out to
  the driver)
- Resolves #1281 (Remove deprecated history output)
- Resolves #1287 (Inconsistent logic for VIC initialization
  can cause crash in debug mode)
- Resolves #1270 (Hexadecimal constants use non-standard
  Fortran)
@billsacks billsacks deleted the ozone_refactor branch February 23, 2021 20:50
ekluzek added a commit to ekluzek/CTSM that referenced this pull request Mar 6, 2021
Refactor ozone code, and misc. small fixes

(1) Restructure ozone code (ESCOMP#1276) in
    preparation for new ozone parameterization.

(2) Fix non-standard hexadecimal constant
    (ESCOMP#1271), needed for gfortran 10

(3) Remove support for CISM1 (ESCOMP#1226)

(4) Move final WaterGridcellBalance call out to clm_driver (resolves
    ESCOMP#1286)

(5) Only add WA and QCHARGE history fields if use_aquifer_layer is true
    (resolves ESCOMP#1281)

(6) Consolidate conditional structures for VIC initialization (resolves
    ESCOMP#1287)

 Conflicts:
	bld/namelist_files/namelist_defaults_ctsm.xml
	bld/unit_testers/build-namelist_test.pl
ekluzek added a commit to olyson/ctsm that referenced this pull request Mar 12, 2021
Refactor ozone code, and misc. small fixes

(1) Restructure ozone code (ESCOMP#1276) in
    preparation for new ozone parameterization.

(2) Fix non-standard hexadecimal constant
    (ESCOMP#1271), needed for gfortran 10

(3) Remove support for CISM1 (ESCOMP#1226)

(4) Move final WaterGridcellBalance call out to clm_driver (resolves
    ESCOMP#1286)

(5) Only add WA and QCHARGE history fields if use_aquifer_layer is true
    (resolves ESCOMP#1281)

(6) Consolidate conditional structures for VIC initialization (resolves
    ESCOMP#1287)

 Conflicts:
	bld/namelist_files/namelist_defaults_ctsm.xml
	src/main/histFileMod.F90
ekluzek added a commit to djk2120/CTSM that referenced this pull request Mar 13, 2021
Refactor ozone code, and misc. small fixes

(1) Restructure ozone code (ESCOMP#1276) in
    preparation for new ozone parameterization.

(2) Fix non-standard hexadecimal constant
    (ESCOMP#1271), needed for gfortran 10

(3) Remove support for CISM1 (ESCOMP#1226)

(4) Move final WaterGridcellBalance call out to clm_driver (resolves
    ESCOMP#1286)

(5) Only add WA and QCHARGE history fields if use_aquifer_layer is true
    (resolves ESCOMP#1281)

(6) Consolidate conditional structures for VIC initialization (resolves
    ESCOMP#1287)

 Conflicts:
	src/biogeophys/CanopyFluxesMod.F90
@sunnivin sunnivin mentioned this pull request May 9, 2021
sunnivin added a commit to sunnivin/CTSM that referenced this pull request May 26, 2021
With the restructuring performed by @billsacks in PR ESCOMP#1276 the restart variables for the o3_coefficients becomes redundant
yelizy pushed a commit to yelizy/ctsm that referenced this pull request Jul 19, 2022
Refactor ozone code, and misc. small fixes

(1) Restructure ozone code (ESCOMP#1276) in
    preparation for new ozone parameterization.

(2) Fix non-standard hexadecimal constant
    (ESCOMP#1271), needed for gfortran 10

(3) Remove support for CISM1 (ESCOMP#1226)

(4) Move final WaterGridcellBalance call out to clm_driver (resolves
    ESCOMP#1286)

(5) Only add WA and QCHARGE history fields if use_aquifer_layer is true
    (resolves ESCOMP#1281)

(6) Consolidate conditional structures for VIC initialization (resolves
    ESCOMP#1287)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants