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

Add MEGAN2 BVOC emission process #111

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

lwcugb
Copy link

@lwcugb lwcugb commented Dec 20, 2024

This pull request adds the MEGAN2.1 BVOC emission scheme based on its implementation in HEMCO.

@bbakernoaa
Copy link
Collaborator

@lwcugb overall I think this looks good. There isn't anything too surprising here. I am wondering if we should add methods for to "accumulate" the values for the historical met data or perhaps store this in the DiagState? @zmoon what do you think?

Copy link
Collaborator

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Just a few initial comments so far.

@lwcugb could you address the compiler warnings associated with your changes? Let me know if you have questions.

Comment on lines 51 to 52
CO2_Inhib_Opt: true
CO2_conc_ppm: 390.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think thus far we've been trying to keep keys all lowercase?

Copy link
Author

Choose a reason for hiding this comment

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

Changed to lowercase

Comment on lines 3 to 9
!! \brief Contains MEGAN2.1 biogenic VOC emission Scheme based on HEMCO and Sam Silva's canopy edits
!!
!! Reference:
!! (1) Guenther et al, (GMD 2012) and associated MEGANv2.1 source code
!! https://doi.org/10.5194/gmd-5-1471-2012
!! (2) Sam Silva's simplified canopy edits (GMD 2020)
!! https://doi.org/10.5194/gmd-13-2569-2020
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this incorporate any of Guenther's standalone MEGAN2.1 code mentioned here? Or is it more based on HEMCO's MEGAN2.1 implementation (in which case HEMCO references and links to GitHub or Zenodo could be added)? And to what extent does Silva's surrogate MEGAN3 come in here?

Copy link
Author

Choose a reason for hiding this comment

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

It is more based on HEMCO and I did not refer to the standalone codes directly. THe HEMCO GitHub link is added now. Silva's edits are all included.

@lwcugb
Copy link
Author

lwcugb commented Jan 15, 2025

Just a few initial comments so far.

@lwcugb could you address the compiler warnings associated with your changes? Let me know if you have questions.

@zmoon Do you mean the warning below? I am not sure how it happens. Could you help me get rid of it?

CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required): Compatibility with CMake < 3.10 will be removed from a future version of CMake. Update the VERSION argument value or use a ... suffix to tell CMake that the project does not need compatibility with older versions.

@zmoon
Copy link
Collaborator

zmoon commented Jan 15, 2025

@zmoon Do you mean the warning below? I am not sure how it happens. Could you help me get rid of it?

Not that one, we can address that later, it's not related to your PR.

I was talking about the warnings from the compiler:

For example with gfortran-11
/home/zmoon/git/CATChem/src/core/config_mod.F90:1420:46:                                                                                                                                                                                                                                                                                                                                                                         1420 |       INTEGER                      :: a_int(4)
      |                                              1
Warning: Unused variable 'a_int' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1432:46:

 1432 |       CHARACTER(LEN=QFYAML_StrLen) :: a_str(2)
      |                                              1
Warning: Unused variable 'a_str' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1414:39:

 1414 |       INTEGER                      :: C
      |                                       1
Warning: Unused variable 'c' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1413:39:

 1413 |       INTEGER                      :: N
      |                                       1
Warning: Unused variable 'n' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1425:52:

 1425 |       CHARACTER(LEN=255)           :: thisLoc,  nLev
      |                                                    1
Warning: Unused variable 'nlev' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1412:46:

 1412 |       INTEGER                      :: nSubStrs
      |                                              1
Warning: Unused variable 'nsubstrs' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1431:53:

 1431 |       CHARACTER(LEN=255)           :: subStrs(MAXDIM)
      |                                                     1
Warning: Unused variable 'substrs' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1428:43:

 1428 |       CHARACTER(LEN=QFYAML_StrLen) :: v_str
      |                                           1
Warning: Unused variable 'v_str' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1423:56:

 1423 |       CHARACTER(LEN=10)            :: xMin_Str, xMax_Str
      |                                                        1
Warning: Unused variable 'xmax_str' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1423:46:

 1423 |       CHARACTER(LEN=10)            :: xMin_Str, xMax_Str
      |                                              1
Warning: Unused variable 'xmin_str' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1424:56:

 1424 |       CHARACTER(LEN=10)            :: yMin_Str, yMax_Str
      |                                                        1
Warning: Unused variable 'ymax_str' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/core/config_mod.F90:1424:46:

 1424 |       CHARACTER(LEN=10)            :: yMin_Str, yMax_Str
      |                                              1
Warning: Unused variable 'ymin_str' declared at (1) [-Wunused-variable]

/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:900:10:

  900 |       IF ( CMLAI == PMLAI ) THEN !(i.e. LAI stays the same)
      |          1
Warning: Equality comparison for REAL(4) at (1) [-Wcompare-reals]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1141:42:

 1141 |       real(fp) :: PAC_DAILY, PHI, BBB, AAA, GAMMA_P_STANDARD
      |                                          1
Warning: Unused variable 'aaa' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1141:37:

 1141 |       real(fp) :: PAC_DAILY, PHI, BBB, AAA, GAMMA_P_STANDARD
      |                                     1
Warning: Unused variable 'bbb' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1162:22:

 1162 |       real(fp) :: EA2L, EA1L, GAMMA_T_LD_SUN, GAMMA_T_LD_SHADE
      |                      1
Warning: Unused variable 'ea2l' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1141:60:

 1141 |       real(fp) :: PAC_DAILY, PHI, BBB, AAA, GAMMA_P_STANDARD
      |                                                            1
Warning: Unused variable 'gamma_p_standard' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1147:37:

 1147 |       real(fp) :: GAMMA_T_LD_STANDARD
      |                                     1
Warning: Unused variable 'gamma_t_ld_standard' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1141:32:

 1141 |       real(fp) :: PAC_DAILY, PHI, BBB, AAA, GAMMA_P_STANDARD
      |                                1
Warning: Unused variable 'phi' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1156:38:

 1156 |       real(fp) :: T_Leaf_Wind_Shade(5)
      |                                      1
Warning: Unused variable 't_leaf_wind_shade' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:1155:36:

 1155 |       real(fp) :: T_Leaf_Wind_Sun(5)
      |                                    1
Warning: Unused variable 't_leaf_wind_sun' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:665:39:

  665 |    subroutine GET_GAMMA_T_LD_C(T, PT_15, PT_24, CT1, CEO, T_Leaf_Int, T_Leaf_Temp, GAMMA_T_LD_C )
      |                                       1
Warning: Unused dummy argument 'pt_15' at (1) [-Wunused-dummy-argument]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:615:32:

  615 |       real(fp), parameter :: z0s = 0.0008467 !< ideal roughness length of soil
      |                                1
Warning: Unused parameter 'z0s' declared at (1) [-Wunused-parameter]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:556:43:

  556 |    subroutine GET_GAMMA_T_LD(T, PT_15, PT_1, CT1, CEO, GAMMA_T_LD)
      |                                           1
Warning: Unused dummy argument 'pt_1' at (1) [-Wunused-dummy-argument]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:529:29:

  529 |       real(fp) :: L_T, L_PT_T    !<
      |                             1
Warning: Unused variable 'l_pt_t' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:149:10:

  149 |       use Error_Mod,     Only : CC_SUCCESS, CC_FAILURE, CC_Error
      |          1
Warning: Unused parameter 'cc_success' which has been explicitly imported at (1) [-Wunused-parameter]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:148:10:

  148 |       use precision_mod, only : fp, ZERO
      |          1
Warning: Unused parameter 'zero' which has been explicitly imported at (1) [-Wunused-parameter]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_bvoc_common_mod.F90:911:63:

  911 |             FNEW = ( TI / DBTWN ) * ( 1.0_fp -  PMLAI / CMLAI )
      |                                                               ^
Warning: 'ti' may be used uninitialized in this function [-Wmaybe-uninitialized]
[ 68%] Building Fortran object src/process/bvoc/CMakeFiles/CATChem_process_bvoc.dir/ccpr_scheme_meganv21_mod.F90.o
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_scheme_meganv21_mod.F90:90:10:

   90 |       USE Constants,     Only : g0, PI_180 ! Example to pull in a constant from the CONSTANTS MODULE < Modify as needed >
      |          1
Warning: Unused parameter 'g0' which has been explicitly imported at (1) [-Wunused-parameter]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_scheme_meganv21_mod.F90:166:33:

  166 |       INTEGER             :: K, S !,DOY  !canopy add and below
      |                                 1
Warning: Unused variable 's' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/ccpr_scheme_meganv21_mod.F90:91:10:

   91 |       use precision_mod, only : fp, ZERO  ! Example to pull in a precision from the PRECISION MODULE < Modify as needed >
      |          1
Warning: Unused parameter 'zero' which has been explicitly imported at (1) [-Wunused-parameter]
[ 70%] Building Fortran object src/process/bvoc/CMakeFiles/CATChem_process_bvoc.dir/CCPr_BVOC_Mod.F90.o
/home/zmoon/git/CATChem/src/process/bvoc/CCPr_BVOC_Mod.F90:201:29:

  201 |       integer            :: c, s, cat_bvoc_idx
      |                             1
Warning: Unused variable 'c' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/CCPr_BVOC_Mod.F90:201:46:

  201 |       integer            :: c, s, cat_bvoc_idx
      |                                              1
Warning: Unused variable 'cat_bvoc_idx' declared at (1) [-Wunused-variable]
/home/zmoon/git/CATChem/src/process/bvoc/CCPr_BVOC_Mod.F90:179:81:

  179 |    SUBROUTINE CCPr_BVOC_Run( MetState, EmisState, DiagState, BvocState, ChemState, RC )
      |                                                                                 1
Warning: Unused dummy argument 'chemstate' at (1) [-Wunused-dummy-argument]
/home/zmoon/git/CATChem/src/process/bvoc/CCPr_BVOC_Mod.F90:179:59:

  179 |    SUBROUTINE CCPr_BVOC_Run( MetState, EmisState, DiagState, BvocState, ChemState, RC )
      |                                                           1
Warning: Unused dummy argument 'diagstate' at (1) [-Wunused-dummy-argument]
/home/zmoon/git/CATChem/src/process/bvoc/CCPr_BVOC_Mod.F90:40:47:

   40 |    SUBROUTINE CCPR_BVOC_Init( Config, ChemState, EmisState, BvocState, RC )
      |                                               1
Warning: Unused dummy argument 'chemstate' at (1) [-Wunused-dummy-argument]

/home/zmoon/git/CATChem/tests/test_bvoc.F90:129:32:

  129 |    subroutine print_info(Config_, BvocState_, MetState_, title_)
      |                                1
Warning: Unused dummy argument 'config_' at (1) [-Wunused-dummy-argument]

@lwcugb
Copy link
Author

lwcugb commented Jan 15, 2025

@zmoon Do you mean the warning below? I am not sure how it happens. Could you help me get rid of it?

Not that one, we can address that later, it's not related to your PR.

I was talking about the warnings from the compiler:

For example with gfortran-11

Oh ok. It seems to have some unused variables. I can clean them. But I guess we should keep the unused 'chemstate' and 'diagstate' for future use?

@zmoon
Copy link
Collaborator

zmoon commented Jan 15, 2025

Oh ok. It seems to have some unused variables. I can clean them. But I guess we should keep the unused 'chemstate' and 'diagstate' for future use?

If it's something you are reasonably sure may be used in the future, you can comment the declaration out instead of fully removing it (but still remove it from arguments if necessary to resolve the warning).

And if you have "equality comparison for real" warning, you can use the rae function from precision_mod to do the comparison instead.

@lwcugb lwcugb requested a review from zmoon January 22, 2025 17:17
@zmoon zmoon force-pushed the feature/catchem_begins branch from 53ad0f7 to 12d57c6 Compare January 22, 2025 19:08
@zmoon zmoon force-pushed the feature/catchem_begins branch from 12d57c6 to 1bda156 Compare January 22, 2025 19:10
Copy link
Collaborator

@zmoon zmoon 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 addressing the warnings!

src/core/config_mod.F90 Outdated Show resolved Hide resolved
@lwcugb
Copy link
Author

lwcugb commented Jan 22, 2025

@lwcugb overall I think this looks good. There isn't anything too surprising here. I am wondering if we should add methods for to "accumulate" the values for the historical met data or perhaps store this in the DiagState? @zmoon what do you think?

@bbakernoaa The historical accumulation function is added with the new commit and saved to DiagState. I think it is better to be saved to restart files if we have that functionality created in the future. The time step is needed to add the historical effect. I am using the MetState TSTEP for now and not sure if a TSTEP is also necessary for EmisState. Another thing I noticed is the temperature historical effect is deactivated by Silva but the historical PAR is still used.

@bbakernoaa
Copy link
Collaborator

@lwcugb I think it will but remember that CATChem is a column model and that the interface will pass that to the host model to be written out. It is also something that could be read in through the host model as well.

TSTEP would be the correct variable.

We could add an option flag to turn the effects on or off. Do you think that would be beneficial?

@lwcugb
Copy link
Author

lwcugb commented Jan 23, 2025

@lwcugb I think it will but remember that CATChem is a column model and that the interface will pass that to the host model to be written out. It is also something that could be read in through the host model as well.

TSTEP would be the correct variable.

We could add an option flag to turn the effects on or off. Do you think that would be beneficial?

Silva's paper suggests that the emissions are only slightly lower. Maybe his other edits make up for the historical temperature or the impact itself is small in the default algorithm (historical PAR may be more important?). So I guess it may not benefit a lot if we keep Silva's method.

@bbakernoaa bbakernoaa self-requested a review January 27, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants