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

Prevent History diagnostic collection duration from being shorter than frequency #2593

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

yantosca
Copy link
Contributor

Name and Institution (Required)

Name: Bob Yantosca
Institution:

Describe the update

This PR addresses the problem reported in #2572 by @Mbavhi1, in which a collection duration setting shorter than the collection frequency, such as:

  SpeciesConc.frequency:      00000001 000000
  SpeciesConc.duration:       00000000 010000

does not halt the run with an error. This can result in empty diagnostic files (as the frequency setting determines the writing of data to file and the duration determines when new files are created).

Expected changes

An error message is now displayed if duration is shorter than frequency, and the run will halt:

===============================================================================
GEOS-Chem ERROR: No diagnostic output will be created for collection: 
"SpeciesConc"!  Make sure that the collection duration setting is not shorter 
than the collection frequency setting in HISTORY.rc!  For example, if the 
frequency is "00000001 000000" (1 day) but the duration is "00000000 010000" 
(1 hour), then this error will occur.
 -> at History_ReadCollectionData (in module History/history_mod.F90)

 -> ERROR occurred at (or near) line     81 of the HISTORY.rc file
===============================================================================

===============================================================================
GEOS-Chem ERROR: Error encountered in "History_ReadCollectionData"!
 -> at History_Init (in module History/history_mod.F90)
===============================================================================

===============================================================================
GEOS-CHEM ERROR: Error encountered in "History_Init"!
STOP at  -> at GEOS-Chem (in GeosCore/main.F90)
===============================================================================
     - CLEANUP: deallocating arrays now...

Related Github Issue

History/history_mod.F90
- Added error trap to make sure that the collection duration (which
  specifies when it's time to write a new file) is never shorter than
  the collection frequency (which specifies the diagnostic time interval).
  This prevents an error that can result in empty diagnostic files.
- Updated error messages to use "YYYYMMDD hhmmss" notation

CHANGELOG.md
- UPdated accordingly

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca added topic: Diagnostics Related to output diagnostic data no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations category: Bug Fix Fixes a previously-reported bug labels Nov 25, 2024
@yantosca yantosca added this to the 14.5.1 milestone Nov 25, 2024
@yantosca yantosca requested a review from msulprizio November 25, 2024 16:57
@yantosca yantosca self-assigned this Nov 25, 2024
@yantosca yantosca changed the base branch from main to dev/no-diff-to-benchmark November 25, 2024 17:31
@yantosca
Copy link
Contributor Author

yantosca commented Nov 25, 2024

All GEOS-Chem Classic integration tests (except tagCO) passed. This is due to a known restart file issue introduced in PR #2510.

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

CodeDir   : a8aeb02 GEOS-Chem update: Merge PR #2578 (Fixes for GCHP adjoint )
GEOS-Chem : 5c8cf0754 Bug fix: Prevent collection duration from being shorter than frequency
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: 59474791
==============================================================================

...
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 identical with the previous integration test except for:

  • TOMAS (parallelization error)
  • APM (parallelization error)
    - gc_4x5_merra2_fullchem_alldiags (numerical noise differences from an unknown source)

We are not sure what is causing the fullchem_alldiags tests to be different.

A rerun of the integration tests showed zero differences in the fullchem_alldiags test.

@yantosca
Copy link
Contributor Author

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     : 5c8cf0754 Bug fix: Prevent collection duration from being shorter than frequency
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: 59476176
==============================================================================

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

All tests were zero-diff w/r/t the prior integration tests.

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.

Looks good to merge. Thanks @yantosca!

@yantosca yantosca merged commit 4a10fac into dev/no-diff-to-benchmark Nov 26, 2024
@yantosca yantosca deleted the bugfix/history-duration branch November 26, 2024 21:18
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 no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations topic: Diagnostics Related to output diagnostic data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

History diagnostic allows for mistake in frequency/duration error
2 participants