-
Notifications
You must be signed in to change notification settings - Fork 322
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
Restructure ozone code #1276
Conversation
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.
src/biogeophys/OzoneMod.F90
Outdated
|
||
select case (this%stress_method) | ||
case (stress_method_lombardozzi) | ||
call this%CalcOzoneStressLombardozzi(bounds, num_exposedvegp, filter_exposedvegp) |
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.
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.
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.
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.
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.
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.
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)
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
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
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
With the restructuring performed by @billsacks in PR ESCOMP#1276 the restart variables for the o3_coefficients becomes redundant
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)
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:
All pass and are bit-for-bit with baselines.