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

Add allocate guards for arrays in pressure_mod #2532

Merged

Conversation

jwallwork23
Copy link
Contributor

Name and Institution (Required)

Name: Joe Wallwork
Institution: University of Cambridge

Describe the update

I noticed that arrays in GeosUtil/pressure_mod.F90 have IF (ALLOCATED(...)) guards before they are deallocated but do not have IF (.NOT. ALLOCATED(...)) guards before they are allocated.

Expected changes

None.

Reference(s)

N/A

Related Github Issue

This fixes a bug that we noticed in the coupling effort for GISS Model E and GEOS-Chem (https://github.com/fetch4/GISS-GC). (See fetch4#7.)

Without these changes, calling INIT_PRESSURE twice will mean arrays are allocated twice, which can cause errors.

@yantosca yantosca self-requested a review October 29, 2024 15:14
@yantosca yantosca added this to the 14.5.1 milestone Oct 29, 2024
@yantosca yantosca self-assigned this Oct 29, 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.

Thanks @jwallwork23. I've added some comments for restructuring the if blocks.

GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
GeosUtil/pressure_mod.F90 Outdated Show resolved Hide resolved
@yantosca yantosca added topic: Structural Modifications Related to GEOS-Chem structural modifications (as opposed to scientific updates) category: Interface to External Model Related to GEOS-Chem updates needed to interface with other models category: Bug Fix Fixes a previously-reported bug and removed category: Bug Fix Fixes a previously-reported bug labels Oct 29, 2024
@jwallwork23 jwallwork23 requested a review from yantosca October 29, 2024 16:49
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! Thanks @jwallwork23

@yantosca
Copy link
Contributor

yantosca commented Nov 19, 2024

@jwallwork23: Would you allow me to push back any merge conflict resolutions to this branch? This will make sure that all of the merge resoluitons are displayed on the PR. That will also bring updates from 14.5.0 into this branch.

@jwallwork23
Copy link
Contributor Author

@jwallwork23: Would you allow me to push back any merge conflict resolutions to this branch? This will make sure that all of the merge resoluitons are displayed on the PR. That will also bring updates from 14.5.0 into this branch.

Sure, that's fine by me @yantosca. I've given you write access to the fork.

Copy link
Contributor

Thanks @jwallwork!

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>
@yantosca yantosca changed the base branch from main to dev/no-diff-to-benchmark November 20, 2024 16:15
@yantosca yantosca merged commit 67ce84a into geoschem:dev/no-diff-to-benchmark Nov 20, 2024
@yantosca
Copy link
Contributor

All GEOS-Chem Classic integration tests passed except tagCO:

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

CodeDir   : e99538a GEOS-Chem update: Merge PR #2532 (Add allocate guards)
GEOS-Chem : cad65f085 Merge PR #2544 (Fixes for SatDiagn diagnostic counters)
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: 58870261
==============================================================================

...

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

The failure is attributed to the date of the restart file for the tagCO simulation having been changed to be consistent with the carbon simulation in PR #2510. This simulation is slated for removal anyway.

@yantosca
Copy link
Contributor

All GCHP integration tests passed:

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

CodeDir       : 8aa7021 GEOS-Chem update: Merge PR #2532 (Add allocate guards)
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     : cad65f085 Merge PR #2544 (Fixes for SatDiagn diagnostic counters)
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: 58871910
==============================================================================

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

@yantosca yantosca added the no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations label Nov 20, 2024
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 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.

2 participants