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

[PULL REQUEST] Change 4D State_Chm%Species array to vector of 3D concentration arrays #990

Merged
merged 13 commits into from May 5, 2022

Conversation

lizziel
Copy link
Contributor

@lizziel lizziel commented Nov 4, 2021

This is a no-diff structural update that changes the container in which we store species concentrations throughout GEOS-Chem. Species concentrations are currently stored in a 4D array called State_Chm%Species which is allocated with dimensions State_Grid%NX, State_Grid%NY, State_Grid%NZ, and State_Chm%nSpecies. This PR changes that to store species concentrations in one 3D array per species (derived type SpcConc). Pointers to the concentration arrays are stored in a 1D vector (State_Chm%SpeciesVec) of size State_Chm%nSpecies. This work is in progress and names are subject to change.

The motivation of this update is to make the species concentration containers the same size as those stored in MAPL when using GEOS or GEOSgcm. Having them the same size will allow (1) pointing to the concentration arrays in the internal state and (2) avoidance of allocating 3D concentration arrays to contain duplicate information. This will reduce the memory requirement for the model. The update to point to the internal state concentration arrays from the new species vector will be a separate PR.

@lizziel lizziel self-assigned this Nov 4, 2021
@lizziel lizziel marked this pull request as draft November 4, 2021 16:50
@lizziel lizziel changed the title [WIP] Change 4D State_Chm%Species array to vector of 3D concentration arrays [WIP] [PULL REQUEST] Change 4D State_Chm%Species array to vector of 3D concentration arrays Nov 4, 2021
@lizziel lizziel added this to the 13.4.0 milestone Nov 4, 2021
@lizziel lizziel added the no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations label Nov 4, 2021
@yantosca
Copy link
Contributor

yantosca commented Nov 4, 2021

Thanks @lizziel, now I understand how this is being implemented. When we add this in, this will likely necessitate a version number change to 14.0.0, as we've broken backwards compatibility.

@stale stale bot added the stale No recent activity on this issue label Dec 5, 2021
@yantosca yantosca added never stale Never label this issue as stale and removed stale No recent activity on this issue labels Dec 6, 2021
@yantosca yantosca modified the milestones: 13.4.0, 14.0.0 Jan 3, 2022
@yantosca yantosca added category: Interface to External Model Related to GEOS-Chem updates needed to interface with other models topic: Structural Modifications Related to GEOS-Chem structural modifications (as opposed to scientific updates) labels Jan 21, 2022
…on arrays

Species_mod now contains type SpcConc which is a pointer to a 3D REAL(fp)
array. State_chm_mod now contains a 1D vector called SpeciesVec that points to
type SpcConc. The 4D array Species is removed. An example of the usage change
is:

old: State_Chm%Species(I,J,L,N)
new: State_Chm%SpeciesVec(N)%Conc(I,J,L)

Where pointer Spc was used locally to point to State_Chm%Species, it now
points to State_Chm%SpeciesVec. For example:

old: Spc(I,J,L,N)
new: Spc(N)%Conc(I,J,L)

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel removed the category: Interface to External Model Related to GEOS-Chem updates needed to interface with other models label Jan 31, 2022
# Conflicts:
#	GeosCore/convection_mod.F90
#	GeosCore/fullchem_mod.F90
#	GeosCore/mercury_mod.F90
#	GeosCore/mixing_mod.F90
#	GeosCore/tpcore_fvdas_mod.F90
#	Headers/state_chm_mod.F90
#	KPP/fullchem/fullchem_SulfurChemFuncs.F90

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@yantosca yantosca added the category: Feature Request New feature or request label Feb 1, 2022
@geoschem geoschem deleted a comment from stale bot Feb 2, 2022
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
The update this commit reverts causes small number differences due to
change in the order of operations in the unit conversions. Removing it
for now allows bitwise zero difference testing for the 4d to 3d species
concentration array update.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
This fix is not ideal because it introduces a large local array in
subroutine TPCORE_WINDOW. However, it eliminates the differences
introduced in the previous implementation using a pointer. The
nested grid advection should be reworked to reduce memory requirement.
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
… sims

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Updates include:

- Remove unused variables and arguments
- Remove binary diagnostic code for global simulations
- Comment out binary diagnostic code for nested grid simulations
  (netcdf diagnostics not yet implemented for nested grid)

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
# Conflicts:
#	GeosCore/global_ch4_mod.F90
#	GeosCore/mercury_mod.F90
#	GeosCore/seasalt_mod.F90
#	GeosCore/tagged_o3_mod.F90
#	GeosCore/vdiff_mod.F90
#	KPP/fullchem/fullchem_SulfurChemFuncs.F90
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
This bug was introduced when adding the q_ptr field within a parallel
do loop to point to the new Species vector containing 3D concentration
arrays.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel
Copy link
Contributor Author

lizziel commented Apr 22, 2022

This PR is now good to go, pending review. I did integration tests for both GCHP and GC-Classic before and after the updates and get zero diffs with the exception of GC-Classic APM (Metrics and SpeciesConc collections) and RRTMG (RRTMG collection only) simulations. I also did two integration test runs of my ref (13.4.0-alpha.27) and compared them. Those also had diffs in the same places, indicating a parallelization issue separate from my updates.

@lizziel lizziel marked this pull request as ready for review April 22, 2022 13:15
@lizziel lizziel changed the title [WIP] [PULL REQUEST] Change 4D State_Chm%Species array to vector of 3D concentration arrays [PULL REQUEST] Change 4D State_Chm%Species array to vector of 3D concentration arrays Apr 22, 2022
@lizziel lizziel requested a review from yantosca April 22, 2022 13:15
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.

This is a straightforward PR that swaps out State_Chm%Species(I,J,L,N) for State_Chm%Species(N)%Conc(I.J,L). Good to merge.

GeosCore/carbon_mod.F90 Show resolved Hide resolved
GeosCore/hco_interface_gc_mod.F90 Show resolved Hide resolved
GeosCore/seasalt_mod.F90 Show resolved Hide resolved
Copy link
Contributor

Great, I thought those were accounted for. Good to go!

@msulprizio msulprizio merged commit cf92308 into dev May 5, 2022
@msulprizio msulprizio deleted the feature/3d_spc_arr branch May 5, 2022 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Feature Request New feature or request never stale Never label this issue as stale no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations topic: Structural Modifications Related to GEOS-Chem structural modifications (as opposed to scientific updates)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants