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.

Sign up for GitHub

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

Jump to bottom

Remove redundant code in hco_extlist_mod.F90 #218

Merged
merged 3 commits into from
Jul 11, 2023
Merged

Remove redundant code in hco_extlist_mod.F90 #218

merged 3 commits into from
Jul 11, 2023

Conversation

yantosca
Copy link
Contributor

@yantosca yantosca commented Jun 20, 2023 •
edited
Loading

Name and Institution (Required)

Name: Bob Yantosca
Institution: Harvard + GCST

Confirm you have reviewed the following documentation

Describe the update

This PR is based on an investigation into geoschem/geos-chem#1827. In routine HEMCO/src/Core/hco_extlist_mod.F90, there are overloaded routines:

  INTERFACE GetExtSpcVal
     MODULE PROCEDURE GetExtSpcVal_Char
     MODULE PROCEDURE GetExtSpcVal_Int
     MODULE PROCEDURE GetExtSpcVal_Sp
  END INTERFACE GetExtSpcVal

which call a common routine GetExtSpcVal_Dr, in which there is a loop over species. For each species, there are IF blocks that test whether the optional output arguments spcScal_sp, spcScal_int, spcScal_char are called.

This is redundant, as the code to populate spcScal_sp can be directly moved into routine GetExtSpcVal_sp, the code to populate spcScal_int can be moved into GetExtSpcVal_int, and the code to populate spcScal_char can be moved into GetExtSpcVal_char. This will allow us to remove the unnecessary GetExtSpcVal_Dr routine.

Expected changes

None, this is a zero-diff update

Reference(s)

N/A

Related Github Issue(s)

See geoschem/geos-chem#1827

Integration tests pending

@yantosca
Remove extraneous routine GetExtSpcVal_Dr in hco_extlist_mod.F90
bf53c91 
src/Core/hco_extlist_mod.F90
- Routine GetExtSpcVal_Dr has a DO loop over species where we test if
  the optional arguments for spcScal_sp, spcScal_int, spcScal_char
  are passed, and if so, then we populate them.
- But this is redundant, as the code to handle spcScal_sp, spcScal_int,
  and spcScal_char can be moved to the overloaded module routines
  (GetExtSpcVal_sp, GetExtSpcVal_int, GetExtSpcVal_char).  We have
  done this.
- Removed GetExtSpcVal_Dr, as it is no longer needed.
- Updated comments, cosmetic changes

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca added category: Feature Request New feature or request topic: Structural Modifications Related to HEMCO structural modifications (as opposed to scientific updates) no-diff-to-benchmark This update will have no impact on benchmark simulations labels Jun 20, 2023
@yantosca yantosca requested a review from msulprizio June 20, 2023 21:35
@yantosca yantosca self-assigned this Jun 20, 2023
@yantosca
Copy link
Contributor Author

yantosca commented Jun 21, 2023 •
edited
Loading

All GEOS-Chem Classic integration tests except TOMAS passed:

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

GCClassic #ae031f8 GEOS-Chem and HEMCO submodule updates: Update transport tracer simulation for consistency with GMAO's tracer gridded component
GEOS-Chem #a89ba6868 Merge PR #1816 (branch 'feature/update-transport-tracers') into dev/14.2.0
HEMCO     #bf53c91 Remove extraneous routine GetExtSpcVal_Dr in hco_extlist_mod.F90

Using 24 OpenMP threads
Number of execution tests: 26

Submitted as SLURM job: 58432379
==============================================================================
 
Execution tests:
------------------------------------------------------------------------------
gc_05x0625_NA_47L_merra2_CH4........................Execute Simulation....PASS
gc_05x0625_NA_47L_merra2_fullchem...................Execute Simulation....PASS
gc_4x5_47L_merra2_fullchem..........................Execute Simulation....PASS
gc_4x5_47L_merra2_fullchem_TOMAS15..................Execute Simulation....FAIL
gc_4x5_47L_merra2_fullchem_TOMAS40..................Execute Simulation....FAIL
gc_4x5_merra2_aerosol...............................Execute Simulation....PASS
gc_4x5_merra2_carbon................................Execute Simulation....PASS
gc_4x5_merra2_CH4...................................Execute Simulation....PASS
gc_4x5_merra2_CO2...................................Execute Simulation....PASS
gc_4x5_merra2_fullchem..............................Execute Simulation....PASS
gc_4x5_merra2_fullchem_aciduptake...................Execute Simulation....PASS
gc_4x5_merra2_fullchem_APM..........................Execute Simulation....PASS
gc_4x5_merra2_fullchem_benchmark....................Execute Simulation....PASS
gc_4x5_merra2_fullchem_complexSOA...................Execute Simulation....PASS
gc_4x5_merra2_fullchem_complexSOA_SVPOA.............Execute Simulation....PASS
gc_4x5_merra2_fullchem_LuoWd........................Execute Simulation....PASS
gc_4x5_merra2_fullchem_marinePOA....................Execute Simulation....PASS
gc_4x5_merra2_fullchem_RRTMG........................Execute Simulation....PASS
gc_4x5_merra2_Hg....................................Execute Simulation....PASS
gc_4x5_merra2_metals................................Execute Simulation....PASS
gc_4x5_merra2_POPs_BaP..............................Execute Simulation....PASS
gc_4x5_merra2_tagCH4................................Execute Simulation....PASS
gc_4x5_merra2_tagCO.................................Execute Simulation....PASS
gc_4x5_merra2_tagO3.................................Execute Simulation....PASS
gc_4x5_merra2_TransportTracers......................Execute Simulation....PASS
gc_4x5_merra2_TransportTracers_LuoWd................Execute Simulation....PASS
 
Summary of test results:
------------------------------------------------------------------------------
Execution tests passed: 24
Execution tests failed: 2
Execution tests not yet completed: 0

The reason for the failure is the update to the TransportTracers. SF6 is now a transport tracer but this conflicts with the TOMAS SF6 sulfate species. A fix for this will be forthcoming later.

===============================================================================
GEOS-Chem ERROR: Is_Gas and Is_Aerosol are both TRUE for species SF6!
 -> at Init_Species_Database (in module Headers/species_database_mod.F90
===============================================================================

===============================================================================
GEOS-Chem ERROR: Error encountered in routine "Init_Species_Database"!
 -> at Init_State_Chm (in module Headers/state_chm_mod.F90)
===============================================================================

===============================================================================
GEOS-Chem ERROR: Error encountered within call to "Init_State_Chm"!
 -> at GC_Init_StateObj (in GeosCore/gc_environment_mod.F90)
===============================================================================

===============================================================================
GEOS-CHEM ERROR: Error encountered in "GC_Init_StateObj!"!
STOP at  -> at GEOS-Chem (in GeosCore/main.F90)
===============================================================================

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

@yantosca
Copy link
Contributor Author

yantosca commented Jun 21, 2023 •
edited
Loading

All GCHP integration tests except TransportTracers passed:

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

GCClassic #318f718 GEOS-Chem and HEMCO submodule updates: Update transport tracer simulation for consistency with GMAO's tracer gridded component
GEOS-Chem #a89ba6868 Merge PR #1816 (branch 'feature/update-transport-tracers') into dev/14.2.0
HEMCO     #bf53c91 Remove extraneous routine GetExtSpcVal_Dr in hco_extlist_mod.F90

Number of execution tests: 5

Submitted as SLURM job: 58434234
==============================================================================
 
Execution tests:
------------------------------------------------------------------------------
gchp_merra2_fullchem................................Execute Simulation....PASS
gchp_merra2_fullchem_benchmark......................Execute Simulation....PASS
gchp_merra2_fullchem_RRTMG..........................Execute Simulation....PASS
gchp_merra2_tagO3...................................Execute Simulation....PASS
gchp_merra2_TransportTracers........................Execute Simulation....FAIL
 
Summary of test results:
------------------------------------------------------------------------------
Execution tests passed: 4
Execution tests failed: 1
Execution tests not yet completed: 0

The error is:

GEOS-Chem ERROR [0011]: The mapData%slot2Id array corresponding to collection "BudgetEmisDryDepFull" 
contains missing values! This can indicate that this collection is either undefined or turned off.  
Please check the HISTORY.rc configuration file in your run directory.
 --> LOCATION:  -> at Get_Mapping (in module Headers/state_diag_mod.F90)
 
GEOS-Chem ERROR [0011]: Error encountered in "Get_Mapping": BudgetEmisDryDepFull
 --> LOCATION:  -> at Get_MapData_and_NumSlots (in module Headers/state_diag_mod.F90)
 
GEOS-Chem ERROR [0011]: Error encountered in "Get_MapData_and_NumSlots"!
 --> LOCATION:  -> at Init_and_Register_R8_3D (in module Headers/state_diag_mod.F90)
 
GEOS-Chem ERROR [0011]: Error encountered in "Init_and_Register", diagID =BudgetEmisDryDepFull
 --> LOCATION:  -> at Init_State_Diag (in Headers/state_diag_mod.F90)
 
GEOS-Chem ERROR [0011]: Error encountered within call to "Init_State_Diag"!
 --> LOCATION:  -> at GC_Init_StateObj (in GeosCore/gc_environment_mod.F90)
pe=00011 FAIL at line=00250    gchp_chunk_mod.F90                       <Error calling GC_Init_StateObj>
pe=00011 FAIL at line=01383    Chem_GridCompMod.F90                     <status=1>
pe=00011 FAIL at line=01807    MAPL_Generic.F90                         <status=1>

Tagging @msulprizio

@yantosca
Copy link
Contributor Author

The error is that the GCHP HISTORY.rc file still uses the old TransportTracers names.

                    'BudgetEmisDryDepFull_Rn222                   ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Pb210                   ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Pb210s                  ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Be7                     ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Be7s                    ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Be10                    ', 'GCHPchem',
                    'BudgetEmisDryDepFull_Be10s                   ', 'GCHPchem',
                    'BudgetEmisDryDepFull_PassiveTracer           ', 'GCHPchem',
                    'BudgetEmisDryDepFull_SF6Tracer               ', 'GCHPchem',
                    'BudgetEmisDryDepFull_CH3ITracer              ', 'GCHPchem',
                    'BudgetEmisDryDepFull_COAnthroEmis25dayTracer ', 'GCHPchem',
                    'BudgetEmisDryDepFull_COAnthroEmis50dayTracer ', 'GCHPchem',
                    'BudgetEmisDryDepFull_COUniformEmis25dayTracer', 'GCHPchem',

This was referenced Jun 21, 2023
Closed
Merged
@yantosca
Copy link
Contributor Author

The GCHP integration test issue should be fixed by geoschem/geos-chem#1840. A new set of integration tests has been submitted.

@yantosca
Additional formatting updates in hco_extlist_mod.F90
a086424 
src/Core/hco_extlist_mod.F90
- In routine GetExtSpcVal_sp, RC is now INTENT(OUT)
- Fixed indentation for END SUBROUTINE statements

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca mentioned this pull request Jun 21, 2023
Closed
@yantosca yantosca linked an issue Jun 21, 2023 that may be closed by this pull request
Scaling issue in HEMCO for extension emissions like fire and soil NOx geoschem/geos-chem#1827
Closed
@msulprizio msulprizio changed the base branch from main to dev/3.7.1 July 11, 2023 14:41
@msulprizio msulprizio added this to the 3.7.1 milestone Jul 11, 2023
msulprizio
msulprizio requested changes Jul 11, 2023
View reviewed changes
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.

CHANGELOG.md needs to be updated. Otherwise, looks good to merge.

@yantosca
Updated CHANGELOG.md w/ info about cleanup of hco_extlist_mod.F90
46f62ed 
CHANGELOG.md
- Added comment about the removal of superfluous routine
  GetExtSpcVal_Dr from src/Core/hco_extlist_mod.F90

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca
Copy link
Contributor Author

Thanks @msulprizio. The changelog has been updated in commit 46f62ed.

@yantosca
Copy link
Contributor Author

This PR has been merged up to the HEMCO 3.7.1 development branch. Integration tests are now running.

@yantosca
Copy link
Contributor Author

yantosca commented Jul 11, 2023 •
edited
Loading

After merging on top of GEOS-Chem PR #1808 and HEMCO PR #215, all GEOS-Chem Classic integration tests passed (except for TOMAS, which will be fixed later).

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

GCClassic #93e58a2 Merge PR #1857 (Update minimum KPP version for fullchem to 3.0.0)
GEOS-Chem #5487d702c Merge PR #1808 (Prevent segfaults in SatDiagn; add SatDiagnEdge)
HEMCO     #bb3b465 Merge PR #218 (Remove redundant code in hco_extlist_mod.F90)

Using 24 OpenMP threads
Number of execution tests: 26

Submitted as SLURM job: 62039887
==============================================================================
 
Execution tests:
------------------------------------------------------------------------------
gc_05x0625_NA_47L_merra2_CH4........................Execute Simulation....PASS
gc_05x0625_NA_47L_merra2_fullchem...................Execute Simulation....PASS
gc_4x5_47L_merra2_fullchem..........................Execute Simulation....PASS
gc_4x5_47L_merra2_fullchem_TOMAS15..................Execute Simulation....FAIL
gc_4x5_47L_merra2_fullchem_TOMAS40..................Execute Simulation....FAIL
gc_4x5_merra2_aerosol...............................Execute Simulation....PASS
gc_4x5_merra2_carbon................................Execute Simulation....PASS
gc_4x5_merra2_CH4...................................Execute Simulation....PASS
gc_4x5_merra2_CO2...................................Execute Simulation....PASS
gc_4x5_merra2_fullchem..............................Execute Simulation....PASS
gc_4x5_merra2_fullchem_aciduptake...................Execute Simulation....PASS
gc_4x5_merra2_fullchem_APM..........................Execute Simulation....PASS
gc_4x5_merra2_fullchem_benchmark....................Execute Simulation....PASS
gc_4x5_merra2_fullchem_complexSOA...................Execute Simulation....PASS
gc_4x5_merra2_fullchem_complexSOA_SVPOA.............Execute Simulation....PASS
gc_4x5_merra2_fullchem_LuoWd........................Execute Simulation....PASS
gc_4x5_merra2_fullchem_marinePOA....................Execute Simulation....PASS
gc_4x5_merra2_fullchem_RRTMG........................Execute Simulation....PASS
gc_4x5_merra2_Hg....................................Execute Simulation....PASS
gc_4x5_merra2_metals................................Execute Simulation....PASS
gc_4x5_merra2_POPs_BaP..............................Execute Simulation....PASS
gc_4x5_merra2_tagCH4................................Execute Simulation....PASS
gc_4x5_merra2_tagCO.................................Execute Simulation....PASS
gc_4x5_merra2_tagO3.................................Execute Simulation....PASS
gc_4x5_merra2_TransportTracers......................Execute Simulation....PASS
gc_4x5_merra2_TransportTracers_LuoWd................Execute Simulation....PASS
 
Summary of test results:
------------------------------------------------------------------------------
Execution tests passed: 24
Execution tests failed: 2
Execution tests not yet completed: 0

Also, most integration tests were identical to GEOS-Chem PR #1808 and HEMCO PR #215, except for those with known parallelization issues. Also note, the TOMAS integration tests list identical but this is because they failed in both Ref & Dev versions.

Checking gc_05x0625_NA_47L_merra2_CH4
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_05x0625_NA_47L_merra2_fullchem
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_47L_merra2_fullchem
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_47L_merra2_fullchem_TOMAS15
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_47L_merra2_fullchem_TOMAS40
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_aerosol
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_carbon
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_CH4
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_CO2
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_aciduptake
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_APM
   -> 3 differences found in OutputDir
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.Metrics.20190701_0000z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.Metrics.20190701_0000z.nc4 
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.SpeciesConc.20190701_0000z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.SpeciesConc.20190701_0000z.nc4 
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/HEMCO_diagnostics.201907010000.nc 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/HEMCO_diagnostics.201907010000.nc 
   -> 1 difference found in Restarts
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_fullchem_APM/Restarts/GEOSChem.Restart.20190701_0100z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_fullchem_APM/Restarts/GEOSChem.Restart.20190701_0100z.nc4 

Checking gc_4x5_merra2_fullchem_benchmark
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_complexSOA
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_complexSOA_SVPOA
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_LuoWd
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_marinePOA
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_fullchem_RRTMG
   -> 1 difference found in OutputDir
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_fullchem_RRTMG/OutputDir/GEOSChem.RRTMG.20190701_0000z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_fullchem_RRTMG/OutputDir/GEOSChem.RRTMG.20190701_0000z.nc4 
   -> No differences in Restarts

Checking gc_4x5_merra2_Hg
   -> 2 differences found in OutputDir
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_Hg/OutputDir/GEOSChem.SpeciesConc.20190101_0000z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_Hg/OutputDir/GEOSChem.SpeciesConc.20190101_0000z.nc4 
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_Hg/OutputDir/HEMCO_diagnostics.201901010000.nc 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_Hg/OutputDir/HEMCO_diagnostics.201901010000.nc 
   -> 1 difference found in Restarts
      * GCC_14.2.1_r7/rundirs/gc_4x5_merra2_Hg/Restarts/GEOSChem.Restart.20190101_0100z.nc4 
        GCC_14.2.1_r8/rundirs/gc_4x5_merra2_Hg/Restarts/GEOSChem.Restart.20190101_0100z.nc4 

Checking gc_4x5_merra2_metals
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_POPs_BaP
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_tagCH4
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_tagCO
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_tagO3
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_TransportTracers
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gc_4x5_merra2_TransportTracers_LuoWd
   -> No differences in OutputDir
   -> No differences in Restarts

@yantosca
Copy link
Contributor Author

After merging on top of GEOS-Chem PR #1808 and HEMCO PR #215, all GCHP integration tests passed (except for TOMAS, which will be fixed later).

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

GCClassic #d023bc5 GEOS-Chem submod update: Merge PR #1808 (SatDiagn diagnostic fixes)
GEOS-Chem #5487d702c Merge PR #1808 (Prevent segfaults in SatDiagn; add SatDiagnEdge)
HEMCO     #bb3b465 Merge PR #218 (Remove redundant code in hco_extlist_mod.F90)

Number of execution tests: 5

Submitted as SLURM job: 62040037
==============================================================================
 
Execution tests:
------------------------------------------------------------------------------
gchp_merra2_fullchem................................Execute Simulation....PASS
gchp_merra2_fullchem_benchmark......................Execute Simulation....PASS
gchp_merra2_fullchem_RRTMG..........................Execute Simulation....PASS
gchp_merra2_tagO3...................................Execute Simulation....PASS
gchp_merra2_TransportTracers........................Execute Simulation....PASS
 
Summary of test results:
------------------------------------------------------------------------------
Execution tests passed: 5
Execution tests failed: 0
Execution tests not yet completed: 0

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

Also, all GCHP integration tests were identical to GEOS-Chem PR #1808 and HEMCO PR #215.

Checking gchp_merra2_fullchem
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gchp_merra2_fullchem_benchmark
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gchp_merra2_fullchem_RRTMG
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gchp_merra2_tagO3
   -> No differences in OutputDir
   -> No differences in Restarts

Checking gchp_merra2_TransportTracers
   -> No differences in OutputDir
   -> No differences in Restarts

@yantosca yantosca merged commit bb3b465 into dev/3.7.1 Jul 11, 2023
@yantosca yantosca deleted the feature/clean-up-hco-extlist-mod branch July 11, 2023 20:46
@yantosca yantosca mentioned this pull request Jul 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msulprizio msulprizio msulprizio approved these changes

Assignees

@yantosca yantosca

Labels
category: Feature Request New feature or request no-diff-to-benchmark This update will have no impact on benchmark simulations topic: Structural Modifications Related to HEMCO structural modifications (as opposed to scientific updates)
Projects
None yet
Milestone
3.7.1
Development

Successfully merging this pull request may close these issues.

Scaling issue in HEMCO for extension emissions like fire and soil NOx
2 participants
@yantosca @msulprizio