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

Fix GEOS-Chem 14.5.0 build in CESM by excluding hard dependencies on HEMCO #2542

Merged

Conversation

jimmielin
Copy link
Contributor

Name and Institution (Required)

Name: Haipeng Lin
Institution: Harvard Univ.

Describe the update

In the CESM model, emissions through HEMCO are provided to GEOS-Chem through the physics buffer and not through the HEMCO subroutine calls. A side-effect of this implementation is that HEMCO versions and GEOS-Chem versions in CESM can be decoupled, and thus hard-dependencies on HEMCO data structures should not apply.

In this particular case, additional ExtState fields in HEMCO are causing CESM build failures in the latest GEOS-Chem version. Most (but not all) subroutines in hco_interface_gc_mod.F90 are unused in CESM and are now blocked off from MODEL_CESM to avoid this hard dependency.

Note: HCOI_GC_WriteDiagn was rearranged in the module but otherwise unchanged, in order to use one #if !defined( MODEL_CESM )

Expected changes

None

Reference(s)

N/A

Related Github Issue

Building CESM with latest GEOS-Chem tag 14.5.0-rc.0

ihough and others added 6 commits October 10, 2024 11:26
Fix bug where download_data.py failed if the dryrun log filename contained uppercase characters
e.g. 'Log.dryrun'. The solution is to not coerce the dryrun log filename to lowercase.
…HEMCO

In the CESM model, emissions through HEMCO are provided to GEOS-Chem through the physics buffer and not through the HEMCO subroutine calls. A side-effect of this implementation is that HEMCO versions and GEOS-Chem versions in CESM can be decoupled, and thus hard-dependencies on HEMCO data structures should not apply.

In this particular case, additional ExtState fields in HEMCO are causing CESM build failures in the latest GEOS-Chem version. Most (but not all) subroutines in hco_interface_gc_mod.F90 are unused in CESM and are now blocked off from MODEL_CESM to avoid this hard dependency.

Signed-off-by: Haipeng Lin <hplin@seas.harvard.edu>
@yantosca yantosca self-assigned this Oct 31, 2024
@yantosca yantosca self-requested a review October 31, 2024 16:07
@yantosca yantosca added category: Interface to External Model Related to GEOS-Chem updates needed to interface with other models topic: CESM Related to running GEOS-Chem in CESM category: Bug Fix Fixes a previously-reported bug and removed category: Bug Fix Fixes a previously-reported bug labels Oct 31, 2024
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.

Looks good @jimmielin. We just need a changelog update.

yantosca and others added 2 commits October 31, 2024 13:15
Headers/state_diag_mod.F90
- Added State_Diag%SatDiagnEdgeCount counter for SatDiagnEdge collection
- Added State_Diag%Archive_SatDiagnEdgeCount logical
- Added State_Diag%Archive_SatDiagnEdge logical
- Allocated State_Diag%SatDiagnPEDGE with vertical dimension
  State_Grid%NZ+1 instead of State_Grid%NZ

GeosCore/diagnostics_mod.F90
- Initialize the State_Diag%SatDiagnEdgeCount counter in the DO loop
  where local time is updated for satellite diagnostics
- Now use State_Grid%NZ+1 vertical levels when saving into the
  State_Diag%SatDiagnPedge field

History/history_util_mod.F90
- Added routine "SatDiagn_or_SatDiagnEdge" to set logical flags to
  determine if the current container is either SatDiagn or SatDiagnEdge

History/history_netcdf_mod.F90
- Call routine SatDiagn_or_SatDiagnEdge to set logical flags indicating
  if the current collection is SatDiagn or SatDiagnEdge
- Added an IF block to compute the average of fields belonging to
  the SatDiagnEdge collection

History/history_mod.F90
- Call routine SatDiagn_or_SatDiagnEdge to set logical flags indicating
  if the current collection is SatDiagn or SatDiagnEdge
- Only reset State_Diag%SatDiagnCount to zero if the current collection
  is "SatDiagn"
- Only reset the "State_Diag%SatDiagnEdgeCount" counter if the current
  collection is "SatDiagnEdge"

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@jimmielin jimmielin requested a review from yantosca November 1, 2024 02:38
@jimmielin
Copy link
Contributor Author

Thanks @yantosca, changelog updated!

@lizziel lizziel self-requested a review November 1, 2024 14:21
CHANGELOG.md Outdated Show resolved Hide resolved
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.

Looks good!

@msulprizio msulprizio changed the base branch from dev/14.5.0 to dev/no-diff-to-benchmark November 1, 2024 18:13
@msulprizio
Copy link
Contributor

Note that I have changed the base branch to dev/no-diff-to-benchmark since this will go into 14.5.1 and not 14.5.0.

yantosca and others added 2 commits November 4, 2024 16:52
CHANGELOG.md
- Updated with info about the satellite diagnostic fixes

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca yantosca added this to the 14.5.1 milestone Nov 7, 2024
This issue happens with Intel 2022.2.1 and possibly others.

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
This merge brings PR geoschem#2560 (Fix trace metals simulation name in
config file comments, by @ihough) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR fixes incorrect comments in configuration files.
Also, merge conflicts in CHANGELOG.md have been resolved.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.ed
This merge brings PR # (Bug fix: allow uppercase log filename
in download_data.py, by @ihough) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR adds a fix so that dry-run log file names that are
fed to the download_data.py script can have mixed case.
Merge conflicts in CHANGELOG.md have also been resolved.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings PR geoschem#2532 (Add allocate guards for
arrays in pressure_mod, by @jwallwork) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR adds checks to make sure that we do not allocate
arrays that have already been allocated.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings PR geoschem#2544 (Add counter for SatDiagnPedge
collection; Prevent satelli-te diagnostic counters from being
inadvertently reset, by @) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR does the following:

1. Adds a separate local time counter
   (State_Diag%SatDiagnEdgeCount) for the SatDiagnEdge
   collection

2. Allocates the State_Diag%SatDiagnPEDGE field with
   State_Diag%NZ+1 vertical levels. (This formerly was
   allocated with State_Diag%NZ levels, which cut off the
   top-of-the atmosphere (L=73) level).

3. Adds a new function SatDiagn_or_SatDiagnEdge in
   History/history_netcdf_mod.F90 to set logical flags to
   denote if the current container name is either
   "SatDiagn" or "SatDiagnEdge"

4. Added code in History/history_netcdf_mod.F90 to compute
   time-averages of fields belonging to the SatDiagnEdge
    collection.

5. Updated code in History/history_mod.F90 to:

   - Reset the State_Diag%SatDiagnCount counter array
     ONLY IF the current container name is SatDiagn.
   - Reset the State_Diag%SatDiagnEdgeCount counter array
     ONLY IF the current container name is SatDiagnEdge.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
run/shared/species_database.yml
- Removed the duplicate WD_RetFactor YAML tag for HgClHO2

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings PR geoschem#2580 (Remove duplicate WD_RetFactor tag
for HgClHO2 entry in species-database.yml, by @yantosca) into
the GEOS-Chem "no-diff-to-benchmark" development stream.

This PR removes a duplicate YAML tag in the species database file.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
This merge brings PR geoschem#2486 (Prevent segmentation fault in
qfyaml with some compilers, by @lizziel) into the GEOS-Chem
"no-diff-to-benchmark" development stream.

This PR fixes a seg fault in Headers/qfyaml_mod.F90 that
has been observed with certain compilers.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca
Copy link
Contributor

@jimmielin: Is it OK if I push resolved merge conflicts back to your branch? This will also bring in the 14.5.0 updates. Just want to make sure that is OK w/ you (and that you're not actively using this branch).

@lizziel
Copy link
Contributor

lizziel commented Nov 21, 2024

@yantosca, looks like this branch has no merge conflicts so should be good to merge.

This merge brings PR geoschem#2542 (Fix GEOS-Chem 14.5.0 build in CESM by
excluding hard dependencies on HEMCO, by @jimmielin) into the
GEOS-Chem "no-diff-to-benchmark" development stream.

This PR removes hard-dependencies on HEMCO when building
GEOS-Chem in the CESM ESM.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca yantosca self-requested a review November 21, 2024 21:38
@yantosca yantosca merged commit e2157dc into geoschem:dev/no-diff-to-benchmark Nov 21, 2024
@yantosca
Copy link
Contributor

All GEOS-Chem Classic integration tests passed, except for tagCO (known restart file issue)

==============================================================================
GEOS-Chem Classic: Execution Test Results

CodeDir   : a8aeb02 GEOS-Chem update: Merge PR #2578 (Fixes for GCHP adjoint )
GEOS-Chem : 62f3db08b Merge PR #2578 (Fixes to compile GCHP adjoint)
HEMCO     : deaa192 HEMCO 3.10.0 release
Cloud-J   : f8a2b7f Update version number for 8.0.1 release
HETP      : 2a99b24 Merge pull request #2 from geoschem/bugfix/initialize_local_variables

Using 24 OpenMP threads
Number of execution tests: 30

Submitted as SLURM job: 59103476
==============================================================================

...
gc_4x5_merra2_tagCO.................................Execute Simulation....FAIL
...
 
Summary of test results:
------------------------------------------------------------------------------
Execution tests passed: 29
Execution tests failed: 1
Execution tests not yet completed: 0

Also all tests were zero-diff w/r/t the previous integration test except for:

  • TOMAS (parallelization issue?)
  • APM (parallelization issue?)

@yantosca
Copy link
Contributor

All GCHP integration tests passed:

==============================================================================
GCHP: Execution Test Results

CodeDir       : 7c2d8e2 GEOS-Chem update: Merge PR #2578 (Fixes for GCHP adjoint )
MAPL          : 9ad63ae Merge PR #37 containing update to vertically flip imports with dimensionless pressure proxy lev coordinates
GMAO_Shared   : 4ddb3ec Merge pull request #2 from geoschem/feature/mapl-upgrade
ESMA_cmake    : ad5deba Added ecbuild as a submodule of ESMA_cmake
gFTL-shared   : 4b82492 Merge branch 'upstream_v1.5.0' into feature/v1.5.0
FMS           : 259759d Merge pull request #3 from geoschem/feature/update_gmao_libs
FVdycoreCubed : af42462 Merge PR #8 (Add PLEadv diagnostic for offline advection in GCHP)
geos-chem     : 62f3db08b Merge PR #2578 (Fixes to compile GCHP adjoint)
HEMCO         : deaa192 HEMCO 3.10.0 release
yaFyaml       : 19afe50 Merge branch 'upstream_v1.0.4' into feature/v1.0.4
pFlogger      : 2c4b724 Merge branch 'upstream_v1.9.1' into feature/v1.9.1
Cloud-J       : f8a2b7f Update version number for 8.0.1 release
HETP          : 2a99b24 Merge pull request #2 from geoschem/bugfix/initialize_local_variables

Number of execution tests: 12

Submitted as SLURM job: 59103750
==============================================================================

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Also, all tests were zero-diff w/r/t the previous set of integration tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Interface to External Model Related to GEOS-Chem updates needed to interface with other models topic: CESM Related to running GEOS-Chem in CESM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants