Move in-module variables to State_Chm_Mod

[jimmielin]

Warning: This update contains very extensive and major structural changes to the GEOS-Chem Core.

This update is the first in a series of significant architectural changes for WRF-to-GC coupling, and more generically, moving GEOS-Chem away from the USE-associated arrays that are generally dependent on (I, J, L) coordinates. Having these arrays inside modules and not inside a "bucket" derived-type object bars running multiple GEOS-Chem instances in the same CPU, as in run-time switching of grid dimensions.

This update moves DRYDEP_MOD's DEPSAV, all allocatable arrays in AEROSOL_MOD, CARBON_MOD, and DIAG_OH_MOD into the State_Chm% derived type object directly. (More modules coming in a future update as I go through all module variables)

• Appropriate module subroutine(s) have been updated to accept State_Chm, if not previously;
• Allocatable arrays in modules have been moved to State_Chm as POINTERs, keeping consistency with existing code; Cleanup_State_Chm has been updated accordingly
• All instances of these arrays in the GEOS-Chem code (in the repository) have been updated to use State_Chm% variants instead

I have tested this update on GCHP, but more extensive testing is probably necessary for this series of updates as they affect a large percentage of GEOS-Chem source code.

Signed-off-by: Haipeng Lin linhaipeng@pku.edu.cn

[msulprizio]

Bob, Lizzie, and I finally had a chance to go through this pull request. We have some thoughts and suggestions:

1. Instead of finding and replacing every instance of an array with State_Chm$array throughout a module, create pointers at the top of each routine (e.g. localarray => State_Chm%array). This is the method we used when replacing the species array (Spc => State_Chm%Species).

2. Where possible, clean up routine arguments. If State_Chm is already passed, then there is no need to also pass State_Chm%array.

3. Consider adding arrays used for bpch diagnostics into State_Diag instead of State_Chm.

4. Try to avoid confusing fields names in the State_Chm object. For example, in DO_DIAG_OH State_Chm%AIR_MASS may be confused with fields in State_Met. If we use pointers at the top of routines, we can still use the existing local array name to avoid having to change code deep in subroutines (e.g. AIR_MASS => State_Chm%Sum_Air_Mass).

5. Allocate State_Chm fields in routine Init_State_Chm instead of local modules.

6. If you aren't already, we highly recommend running unit tests and difference tests often as you make these updates. Unit tests will ensure you don't break any simulations and difference tests will ensure you aren't introducing any changes to model output.

@yantosca @lizziel Feel free to add more if I forgot anything!

@jimmielin - We're happy to continue the discussion and/or assist you with some of these updates. Please let us know what works for you.

[yantosca]

Hi Haipeng,

Thanks for the pull request, this is a great step. To expand on the point #1 that Melissa made,
``'
! In declaration section
REAL(fp), POINTER :: DepSav(:,:,:)
...
! At start of routine
DEPSAV => State_Chm%DepSav
...
! Dry deposition active for both POP-Gas and POP-Particle;
! pass drydep frequency to CHEM_POPGP (NOTE: DEPSAV has units 1/s)
CALL CHEM_POPGP( DEPSAV(:,:,dd_POPG),
... etc ...
...
! At end of routine
DEPSAV => NULL()

That will minimize the number of changes to the source code, which should reduce merge conflicts.

Great work!  Thanks a bunch,

Bob Y.

[lizziel]

@jimmielin
Thanks for doing this work! Another thing that came up in recent discussions is preservation of the TOMAS aerosol microphysics option in GEOS-Chem. We request that you run the TOMAS15 unit test to ensure that the option still compiles and runs. TOMAS40 has an existing issue causing concentrations to blow up so that one should be turned off.

[jimmielin]

Thanks all for the comments and thoughts. It indeed makes sense to use pointers on top of subroutines to minimize code changes. I'll rework the patches to do this as it seems more in-line with the migration procedures for State_Chm%Species.

@msulprizio @yantosca @lizziel
Regarding 4. I was wondering if we should set up a naming convention for these new variables? For example, if it's for aerosol, we could name it State_Chm%Aerosol_PM25 so we know that it's a variable used and primarily set in that module?

I agree with 5 but not all modules have their use-associated arrays as dimension-dependent (some are fixed rates for species, especially for drydep_mod). Does this mean that for modules that have all their arrays migrated to State_Chm (and/or State_Diag) we should get rid of their Init_ and Cleanup_ methods, otherwise keep these methods?

I will work on the changes and make sure to test them with difference tests and unit tests, to make sure there is no impact to the overall output. Thanks all for your suggestions!

[msulprizio]

Bob, Lizzie, and I met yesterday to discuss setting up a naming convention for new variables in State_Chm. However, we decided it's probably easiest to use your/our best judgement for now. In general, we use camel case names and try to be as concise as possible. We also want to make sure the names aren't too similar to other fields in State_Chm, and if they are to try to make one or both of those names more descriptive. For example, I have moved several module variables to State_Chm to facilitate saving those fields to the restart (this will go into 12.1.0). The old and new names are summarized below:

• H2O2s from sulfate_mod.F is now State_Chm%H2O2AfterChem
• SO2s from sulfate_mod.F is now State_Chm%SO2AfterChem
• HSAVE_KPP from flexchem_mod.F90 is now State_Chm%KPPHvalue
• DRY_TOTN from get_ndep_mod.F is now State_Chm%DryDepNitrogen
• WET_TOTN from get_ndep_mod.F is now State_Chm%WetDepNitrogen

I don't think my changes above conflict with any of the changes you've made in this pull request, however there may be other updates going into 12.1.0 that remove module-level arrays. Therefore, it might make sense to move these updates/this pull request to a branch off of master once 12.1.0 is released so you have all of these updates. We'd like to eventually remove the Dev branch, off of which this pull request is based but we can't do that until this request is closed. We've essentially abandoned Dev and moved to a new branch naming convention using dev/12.0.3, dev/12.1.0, feature/FlexGrid, etc.

[jimmielin]

Hi Melissa & GCST,

Thanks for the update! Due to the new coding & branch requirements and probably a newer naming scheme, it might make sense to close this pull request without merging so you can remove the Dev branch, and then I can open a new one based off a future version so the commit log is uncluttered with older versions of the code.

There is a minor request I'd like to mention here that's related to removal of the Dev branch: there are a few commits regarding WRF-GC integration that are right now in Dev but not in 12.0.3 or any of the 12.0.x point releases, this prevents us from merging the 12.0.x bug fixes directly into WRF-GC (as they're also not in the Dev or dev/12.1.0 branches). Would it be possible to merge these changes (under MODEL_WRF) into a point release or the current in-development branch (dev/12.1.0 maybe?), so we can point WRF-GC's copy of GEOS-Chem to that branch and keep in sync with all the development work?

Thanks!

[msulprizio]

Hi Haipeng,

We can certainly pull in your MODEL_WRF updates off of the Dev branch. If they're ready now then we can bring those update into dev/12.1.0 which we are aiming to wrap up this week. Otherwise, they can go into the following 12.1.z version.

The rest of your plan - to close this pull request and create a new one off of the latest development branch - sounds good to us. Thanks!

[jimmielin]

Hi Melissa,

Sure, please do pull the updates into dev/12.1.0, and once that's done I'll pull 12.1.0 into WRF-GC.

I'll close this pull request and open a new one as updates for moving State_Chm are rewritten based on the new requirements. Thanks!

Haipeng

[msulprizio]

Hi Haipeng,

The MODEL_WRF, NO_EXE, etc. updates are already merged into dev/12.1.0. We will be bringing the WRF-GC updates into 12.1.0, so these updates are not in the master branch (12.0.3) yet. You should be able to work with the dev/12.1.0 branch until that version has been benchmarked and released.

Best,
Melissa

[lizziel]

I am revisiting this old PR since I am looking at aerosol_mod.F90 module variables and thinking we should move them to State_Chm. I searched around and found this old work. @jimmielin, did you ever revisit this?

[jimmielin]

Hi @lizziel, I don't think I've redone the work in aerosol_mod.F90 in particular but a lot of the modules now have their variables moved to State_Chm. Perhaps we need to do a final sweep and finish the rest.

[lizziel]

I'll make the updates in aerosol_mod.F90 as part of my Cloud-J work.