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 several issues with the GEOS-Chem Classic satellite diagnostics (SatDiagn and SatDiagnEdge collections) #2369

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

yantosca
Copy link
Contributor

Name and Institution (Required)

Name: Bob Yantosca
Institution: Harvard + GCST

Describe the update

This is the companion PR to #2368 and #2017. We have fixed several issues with the GEOS-Chem Classic satellite diagnostics, namely:

  1. We have moved the IF block that resets the satellite diagnostic counter array (State_Diag%SatDiagnCount) to zero out of History_Netcdf_Write (in History/history_netcdf_mod.F90) to the end of the History_Write routine (in History/history_mod.F90), outside of the loop over container names. Because there are now two satellite diagnostic containers (SatDiagn and SatDiagnEdge), the counter array was being reset after the first container was written out to disk.

  2. We have removed the SatDiagnPEDGE entry from the SatDiagn collection in all GEOS-Chem Classic HISTORY.rc.* template files. This needs to go into the SatDiagnEdge collection.

  3. We have fixed a typo in the SatDiagnEdge collection in all GEOS-Chem Classic HISTORY.rc.* template files. The erroneous entry was SatDiagnConc_PEDGE, which should be SatDiagnPEDGE.

  4. The SatDiagnOH field now works properly for the carbon simulation (which has the species name FixedOH instead of `OH).

Expected changes

The satellite diagnostics should now work as expected for all simulations.

Related Github Issue

History/history_netcdf_mod.F90
- Moved the zeroing of State_Diag%SatDiagnCount to routine
  History_Write in history_mod.F90
- Also now test if the container name is either SatDiagn or SatDiagnEdge
  only once at the top of History_Netcdf_Write.

History/history_mod.F90
- Reset State_Diag%SatDiagnCount outside of the loop over containers,
  after History_Netcdf_Write has been called.  This is because we now
  have separate SatDiagn and SatDiagnEdge collections that both use
  the same SatDiagnCount array.

run/GCClassic/HISTORY.rc.templates/HISTORY.rc.*
- Remove `SatDiagnPEDGE` from the `SatDiagn` collection; this needs to
  be in the separate `SatDiagnEdge` collection
- Fix typo "SatDiagnConc_PEDGE" -> "SatDiagnPEDGE"
- Remove fields from SatDiagn entries that are not relevant
  for the given simulation (the user can restore these)

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca added topic: Diagnostics Related to output diagnostic data category: Bug Fix Fixes a previously-reported bug labels Jul 10, 2024
@yantosca yantosca added this to the 14.4.2 milestone Jul 10, 2024
@yantosca yantosca requested a review from msulprizio July 10, 2024 16:10
@yantosca yantosca self-assigned this Jul 10, 2024
@yantosca yantosca linked an issue Jul 10, 2024 that may be closed by this pull request
@yantosca yantosca linked an issue Jul 10, 2024 that may be closed by this pull request
@yantosca
Copy link
Contributor Author

All GEOS-Chem Classic integration tests passed:

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

GCClassic #c49fcec Submod updates: Merge GEOS-Chem PR #2353 and Cloud-J PR #19
GEOS-Chem #cce6dc5b4 Fixed multiple issues with satellite diagnostics
HEMCO     #2192e0e HEMCO 3.9.1 release

Using 24 OpenMP threads
Number of execution tests: 28

Submitted as SLURM job: 39319517
==============================================================================
 
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

All tests were zero-diff w/r/t PR #2353 except for:

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

@yantosca
Copy link
Contributor Author

All GCHP integration tests passed:

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

GCHP      #493b4fd Submod updates: Merge GEOS-Chem PR #2353 and Cloud-J PR #19
GEOS-Chem #cce6dc5b4 Fixed multiple issues with satellite diagnostics
HEMCO     #

Number of execution tests: 11

Submitted as SLURM job: 39319592
==============================================================================
 

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

Also all tests were zero-diff w/r/t PR #2353.

Copy link
Contributor

@msulprizio msulprizio left a comment

Choose a reason for hiding this comment

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

Thanks, Bob! This looks great.

@yantosca yantosca merged commit 7e40016 into dev/no-diff-to-benchmark Jul 11, 2024
@yantosca yantosca deleted the bugfix/satellite-diagnostics branch July 11, 2024 14:50
msulprizio added a commit that referenced this pull request Jul 19, 2024
We now run a 2x2.5 ModelE2.1 (aka GCAP 2.0) full-chemistry simulation in
the GCClassic integration tests. Only one scenario (SSP2-4.5) is evaluated
here simply to ensure these future scenario simulations compile and run
for 1 hour successfully. I will defer to @ltmurray for guidance on whether
additional simulations are needed.

The GCAP 2.0 integration test is passing off of 14.4.1, confirming that the
fixes in #2342 should have resolved
the issues of GCAP 2.0 not working in 14.0.0 (as reported by Lee Murray at
IGC11).

Sample integration test output:

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

GCClassic #c49fcec Submod updates: Merge GEOS-Chem PR #2353 and Cloud-J PR #19
GEOS-Chem #7e4001658 Merge PR #2369 (Fix several issues with satellite diagnostics)
HEMCO     #2192e0e HEMCO 3.9.1 release

Using 24 OpenMP threads
Number of execution tests: 29

Submitted as SLURM job: 40462513
==============================================================================

Execution tests:
------------------------------------------------------------------------------
gc_05x0625_NA_47L_merra2_CH4........................Execute Simulation....PASS
gc_05x0625_NA_47L_merra2_fullchem...................Execute Simulation....PASS
gc_2x25_ModelE2.1_fullchem..........................Execute Simulation....PASS
```

Signed-off-by: Melissa Sulprizio <[email protected]>
lizziel pushed a commit that referenced this pull request Jul 30, 2024
We now run a 2x2.5 ModelE2.1 (aka GCAP 2.0) full-chemistry simulation in
the GCClassic integration tests. Only one scenario (SSP2-4.5) is evaluated
here simply to ensure these future scenario simulations compile and run
for 1 hour successfully. I will defer to @ltmurray for guidance on whether
additional simulations are needed.

The GCAP 2.0 integration test is passing off of 14.4.1, confirming that the
fixes in #2342 should have resolved
the issues of GCAP 2.0 not working in 14.0.0 (as reported by Lee Murray at
IGC11).

Sample integration test output:

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

GCClassic #c49fcec Submod updates: Merge GEOS-Chem PR #2353 and Cloud-J PR #19
GEOS-Chem #7e4001658 Merge PR #2369 (Fix several issues with satellite diagnostics)
HEMCO     #2192e0e HEMCO 3.9.1 release

Using 24 OpenMP threads
Number of execution tests: 29

Submitted as SLURM job: 40462513
==============================================================================

Execution tests:
------------------------------------------------------------------------------
gc_05x0625_NA_47L_merra2_CH4........................Execute Simulation....PASS
gc_05x0625_NA_47L_merra2_fullchem...................Execute Simulation....PASS
gc_2x25_ModelE2.1_fullchem..........................Execute Simulation....PASS
```

Signed-off-by: Melissa Sulprizio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Bug Fix Fixes a previously-reported bug topic: Diagnostics Related to output diagnostic data
Projects
None yet
2 participants