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 minor issues that were not addressed in PR #226 (the netCDF-F90 update) #227

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

yantosca
Copy link
Contributor

@yantosca yantosca commented Jul 18, 2023

Name and Institution (Required)

Name: Bob Yantosca
Institution: Harvard + GCST

Confirm you have reviewed the following documentation

Describe the update

This PR fixes several issues with the prior PR #226. In particular:

  • The hco_m_netcdf_io_write.F90 was never updated to use netCDF-F90 library functions; this is now fixed.
  • Replaced lingering netCDF-F77 library calls with netCDF-F90 equivalents
  • Fixed typos and removed extraneous code

Expected changes

This is a zero-diff update, as it does not touch any science code.

Reference(s)

N/A

Related Github Issue(s)

Closes #225

yantosca added 3 commits July 18, 2023 17:14
src/Shared/NcdfUtil/hco_m_netcdf_io_read.F90
- Remove ELSE blocks in the IF ( dostop ) teset
- Fix indentation
- Change a leftover instance of NF_NOERR to NF90_NOERR
- Update "Revision History" comments in subroutine headers

src/Shared/NcdfUtil/hco_m_netcdf_io_write_mod.F90
- This file was not converted to use the netCDF-F90 interface,
  but this has now been resolved.
- Use F90 indentation

src/Shared/NcdfUtil/hco_ncdf_mod.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_checks.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_get_dimlen.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_handle_err.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_create.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_define.F90
src/Shared/NcdfUtil/hco_m_netcdf_io_readattr.F90
- Update "Revision History" subroutine headers

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
src/Shared/hco_m_netcdf_io_read.F90
- Extraneous code was introduced into this module.  This has now
  been removed.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
src/Shared/NcdfUtil/hco_m_netcdf_io_write.F90
- Replaced "NF90_Put_Var_Double" (which is not a netCDF-F90 function)
  with the proper function call "NF90_Put_Var".  This was leftover
  when we were replacing text.

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@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 Jul 18, 2023
@yantosca yantosca requested a review from lizziel July 18, 2023 21:44
@yantosca yantosca self-assigned this Jul 18, 2023
@yantosca
Copy link
Contributor Author

yantosca commented Jul 19, 2023

After merging on top of PR #226 and PR geoschem/geos-chem#1879, all GEOS-Chem Classic integration tests passed (except TOMAS, which is a known issue):

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

GCClassic #2773dc8 GEOS-Chem submod updates: Set ESMF logging mode at run-time
GEOS-Chem #5c8050f08 Merge PR #1882 (Convert GEOS-Chem netCDF routines to use netCDF-F90)
HEMCO     #599775d Merge PR #227 (Fix several minor issues not addressed in PR #226)

Using 24 OpenMP threads
Number of execution tests: 26

Submitted as SLURM job: 63552572
==============================================================================
 
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

Furthermore, most integration tests were identical to PR #1879. The exceptions are:

  • TOMAS (failed integration tests)
  • APM (known parallelization issue, will be fixed later)
  • RRTMG diagnostic output (known parallelization issue, will be fixed later)
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
   -> 2 differences found in OutputDir
      * GCC_14.2.1_r12/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.Metrics.20190701_0000z.nc4 
        GCC_14.2.1_r13/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.Metrics.20190701_0000z.nc4 
      * GCC_14.2.1_r12/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.SpeciesConc.20190701_0000z.nc4 
        GCC_14.2.1_r13/rundirs/gc_4x5_merra2_fullchem_APM/OutputDir/GEOSChem.SpeciesConc.20190701_0000z.nc4 
   -> 1 difference found in Restarts
      * GCC_14.2.1_r12/rundirs/gc_4x5_merra2_fullchem_APM/Restarts/GEOSChem.Restart.20190701_0100z.nc4 
        GCC_14.2.1_r13/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_r12/rundirs/gc_4x5_merra2_fullchem_RRTMG/OutputDir/GEOSChem.RRTMG.20190701_0000z.nc4 
        GCC_14.2.1_r13/rundirs/gc_4x5_merra2_fullchem_RRTMG/OutputDir/GEOSChem.RRTMG.20190701_0000z.nc4 
   -> No differences in Restarts

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

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

yantosca commented Jul 19, 2023

After merging on top of PR #226 and PR geoschem/geos-chem#1879, all GCHP integration tests passed:

GCHP: Execution Test Results

GCClassic #5994b33 Merge PR #330 (Set ESMF logging option at run-time via config file)
GEOS-Chem #5c8050f08 Merge PR #1882 (Convert GEOS-Chem netCDF routines to use netCDF-F90)
HEMCO     #599775d Merge PR #227 (Fix several minor issues not addressed in PR #226)

Number of execution tests: 5

Submitted as SLURM job: 63553029
==============================================================================
 
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!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Furthermore, all GCHP integration tests were zero-diff w/r/t PR #1879:

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 added this to the 3.7.1 milestone Jul 19, 2023
@lizziel lizziel changed the base branch from main to dev/3.7.1 July 19, 2023 20:30
Copy link
Contributor

@lizziel lizziel 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. I generally leave the initial version line for the revision history to give the original author credit, and to see when the file originated at a glance. The GitHub history wouldn't have this as the original commit since HEMCO used to be in the GEOS-Chem repo. But I think it's okay to merge this anyway.

Sorry I missed in my other review that there was another file that needed the changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

Need to update HEMCO I/O routines to use the netCDF-F90 interface
2 participants