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

[WIP] [Pull Request] Remove non-zero diffs in GCHP introduced by splitting up simulations in time #1198

Closed
wants to merge 18 commits into from

Conversation

lizziel
Copy link
Contributor

@lizziel lizziel commented Mar 28, 2022

This PR removes the long-standing problem in GCHP of generating different output data depending on how simulations are split up in time. All fixes extend to GC-Classic as well, but the analogous problem in GC-Classic is not corrected with this pull request. Updates to correct the problem in GC-Classic will be submitted as a separate pull request, possibly for a later version.

Updates in this PR consist of several unrelated bug fixes that each introduce differences upon simulation restart. Changes include:

  1. Reorder several calls in chemistry, specifically moving the calls to Calc_Strat_Aer, Aerosol_Conc, and RDAER to before the first call to ChemSulfate in Do_Chemistry. More generally, the aerosol chemistry is now out of Do_Fullchem and is in Do_Chemistry instead, including the call to compute dust online. This update allows State_Chm%AeroArea and State_Chm%AreaRadi to be computed in the same timestep that they are used in ChemSulfate. Prior to this change they were used in ChemSulfate as zero in the first timestep. Including them in the restart file would also solve this issue, but that would be 28 additional 3D fields in the GCHP and GEOS internal state which is too costly at high resolution.

  2. Move Isorropia call to before the call to do_fullchem. Several Isorropia output arrays are used in KPP but previously Isorropia was called after KPP, resulting in zero values used in the first timestep. These arrays were previously added to the GCHP and GEOS restart files, but moving the call to Isorropia reduces the number of restart fields required to a single 3D array.

  3. Add new restart variable AeroH2O_SNA which is computed in Isorropia but used in subroutine RDAER.

  4. Remove skipping wet settling when first timestep. This was done in both the stratosphere in Calc_Strat_Aer and the troposphere in Grav_Settling.

  5. Add State_Chm arrays JOH and JNO2 for surface J-values of RXN_O3_1 and RXN_NO2, and add them as new restart variables. These correspond to reactions O3 + hv --> O2 + O and NO2 + hv --> NO + O respectively. The surface J-values are used in the HEMCO paranox extension which is called before J-values are calculated. This resulted in used zero values in the first timestep.

  6. Move carbon_mod module variable ORVC_SESQ to State_Chm array ORVCsesq, and store the array as a restart variable for both GCHP and GC-Classic. This array was already carried in the restart file in GEOS. It is the sesquiterpenes mass per grid box and is used in SOA chemistry prior to it being set. This caused differences in ASOA*, ASOG*, TSOA*, and TSOG* concentrations.

  7. Compute cos SZA before emissions are run. This is done in GEOS but previously was not done in GCHP. I did not isolate this difference to see if it was actually a bug.

  8. Set the TotalOC diagnostic to zero before it is set every timestep. If POA or OPOA are true then TotalOC gets reset every timestep. If neither are true, and LSOA is true, then TotalOC is increased from its current value. The problem with this is we do not set State_Diag%TotalOC to zero prior this computation. This causes TotalOC to be the cumulative value rather than the instantaneous value if POA and OPOA are false and LSOA is true. This causes erroneous values, and also makes the values different depending on how you split up your run into multiple jobs.

The fixes, combined with a couple fixes that went in 13.4.0 (geoschem/HEMCO#140 and #1178) make GCHP benchmark simulation output the same regardless of how the run is split up. Differences remain in GC-Classic indicating the problem in GC-Classic is likely from advection and/or I/O.

For more information on the impact of this problem and the fixes, see http://ftp.as.harvard.edu/gcgrid/geos-chem/validation/GC_time_regression_fixes/.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
…istry

Calls include Calc_Strat_Aer, Aerosol_Conc, RDAER, and RDust_Online.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
…timers

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Note that this array is never actually used and will later be deleted
during cleanup.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
…iables

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
There are now ten internal state arrays beyond species concentrations
and HEMCO restart variables that are on by default for GCHP. Nine of these
were already present before this update. One additional one is now added
to avoid differences in chemistry when splitting up a run. This new
array is AeroH2O_SNA which corresponds to State_Chm%AeroH2O(:,:,:,NDUST+1)
which is computed in isorropia and used in RDAER and full chemistry.

The JNO2 and JOH arrays in the internal state are included for further
testing.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel added the category: Bug Something isn't working label Mar 28, 2022
@lizziel lizziel added this to the 14.0.0 milestone Mar 28, 2022
@lizziel lizziel self-assigned this Mar 28, 2022
# Conflicts:
#	GeosCore/chemistry_mod.F90
#	Interfaces/GCHP/gchp_chunk_mod.F90

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel changed the title [WIP] Bugfix/gchp time regression [WIP] Remove non-zero diffs introduced by splitting up simulations in time Mar 28, 2022
lizziel and others added 7 commits March 31, 2022 14:14
This internal state field is declared and set separately from GCHP due to
a precision difference. GCHP explicitly sets internal state fields as
REAL8 while GEOS should not.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
GeosCore/chemistry_mod.F90
- There were leftover "goto 9999" and "9999 continue" statements that
  were used for debugging parallelization issues.  While these had been
  removed in a prior commit, they snuck back into the code in a
  subsequent commit.  Therefore we are removing them again.  This
  had caused aerosol concentrations to be incorrect in GC benchmarks.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
Using these state_chm arrays introduces differences in the restart file.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Using these fixes the multi vs single run differences in GCHP that occur
in paranox. However, the restarts files show different values for
the new arrays, State_Chm%JOH and State_Chm%JNO2. This needs to be looked
at further.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@yantosca yantosca self-requested a review April 8, 2022 14:57
Copy link
Contributor

@yantosca yantosca left a comment

Choose a reason for hiding this comment

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

These updates are OK to merge. Further modifications may be necessary, pending testing and validation.

GeosCore/chemistry_mod.F90 Show resolved Hide resolved
GeosCore/fullchem_mod.F90 Show resolved Hide resolved
Interfaces/GCHP/Chem_GridCompMod.F90 Show resolved Hide resolved
@yantosca yantosca modified the milestones: 14.0.0, 13.4.0 Apr 8, 2022
@yantosca yantosca closed this Apr 8, 2022
@yantosca yantosca deleted the bugfix/gchp_time_regression branch April 8, 2022 19:19
@yantosca yantosca restored the bugfix/gchp_time_regression branch April 8, 2022 19:30
@lizziel lizziel changed the title [WIP] Remove non-zero diffs introduced by splitting up simulations in time [WIP] Remove non-zero diffs in GCHP and GEOS introduced by splitting up simulations in time Apr 22, 2022
@lizziel lizziel changed the title [WIP] Remove non-zero diffs in GCHP and GEOS introduced by splitting up simulations in time [WIP] [Pull Request] Remove non-zero diffs in GCHP introduced by splitting up simulations in time Apr 22, 2022
@geoschem geoschem deleted a comment from yantosca Apr 22, 2022
@lizziel lizziel modified the milestones: 13.4.0, 14.0.0 Apr 22, 2022
@lizziel
Copy link
Contributor Author

lizziel commented Apr 26, 2022

This PR is superseded by #1229.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants