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

History diagnostics now pass arrays with a singleton dimension for time to netCDF-F90 library functions #1896

Merged
merged 2 commits into from
Aug 1, 2023
Merged

History diagnostics now pass arrays with a singleton dimension for time to netCDF-F90 library functions #1896

merged 2 commits into from
Aug 1, 2023

Conversation

yantosca
Copy link
Contributor

@yantosca yantosca commented Aug 1, 2023

Name and Institution (Required)

Name: Bob Yantosca
Institution: Harvard + GCST

Confirm you have reviewed the following documentation

Describe the update

This PR fixes the problem with archiving hourly diagnostics via GEOS-Chem Classic History diagnostics that was raised by @nicholasbalasus in issue #1888.

The fix involves using arrays with a singleton time dimension, as shown from this snippet from history_netcdf_mod.F90:

          !------------------------------------------------------------------
          ! 3-D data
          !------------------------------------------------------------------
          CASE( 3 )

             ! Get dimensions of data
             Dim1 = SIZE( Item%Data_3d, 1 )
             Dim2 = SIZE( Item%Data_3d, 2 )
             Dim3 = SIZE( Item%Data_3d, 3 )

             ! Get average for satellite diagnostic:
             IF ( Container%name == 'SatDiagn' ) THEN
                Item%Data_3d = Item%Data_3d / State_Diag%SatDiagnCount
                Item%nUpdates = 1.0
             ENDIF

             ! Allocate the 4-byte or 8-byte output array
             ! Need a singleton 4th dimension for netCDF-F90
             IF ( output4bytes ) THEN
                ALLOCATE( NcData_4d4( Dim1, Dim2, Dim3, 1 ), STAT=RC )
             ELSE
                ALLOCATE( NcData_4d8( Dim1, Dim2, Dim3, 1 ), STAT=RC )
             ENDIF

             ! Copy or average the data and store in a 4-byte or 8-byte array
             IF ( Item%Operation == COPY_FROM_SOURCE ) THEN

                !%%% Instantaneous output %%%
                IF ( output4Bytes ) THEN
                   NcData_4d4(:,:,:,1) = Item%Data_3d
                ELSE
                   NcData_4d8(:,:,:,1) = Item%Data_3d
                ENDIF
                Item%Data_3d  = 0.0_f8
                Item%nUpdates = 0.0_f8

             ELSE

                !%%% Time-averaged output %%%
                Item%Data_3d  = Item%Data_3d / Item%nUpdates
                IF ( output4Bytes ) THEN
                   NcData_4d4(:,:,:,1) = Item%Data_3d
                ELSE
                   NcData_4d8(:,:,:,1) = Item%Data_3d
                ENDIF
                Item%Data_3d  = 0.0_f8
                Item%nUpdates = 0.0_f8

             ENDIF

             ! Compute start and count fields
             St4d = (/ 1,    1,    1,    Container%CurrTimeSlice /)
             Ct4d = (/ Dim1, Dim2, Dim3, 1                       /)

             ! Write data to disk and deallocate output array
             IF ( output4bytes ) THEN
                CALL NcWr( NcData_4d4, NcFileId, Item%Name, St4d, Ct4d )
                DEALLOCATE( NcData_4d4, STAT=RC )
             ELSE
                CALL NcWr( NcData_4d8, NcFileId, Item%Name, St4d, Ct4d )
                DEALLOCATE( NcData_4d8, STAT=RC )
             ENDIF

Thus we now

  • Copy 3D data into a 4D array with a singleton time dimension
  • Copy 2D data into a 3D array w/ a singleton time dimension (not shown)
  • Copy 1D data into a 2D array w/ a singleton time dimension (not shown)

before passing to the routine NcWr, which calls netCDF-F90 library routines.

NOTE: this was not a problem with the netCDF-F77 interface, which didn't require the singleton time dimension.
In this case we copy 3D data into a 4D array with a singleton dimension for time before passing it to the netCDF routines.

Expected changes

Before this fix, all time slices after the first time slice were missing when using the 14.2.1 development stream. This was not present in prior versions.:

image

Applying the fix in this PR removes the error:
image
run.txt

Reference(s)

N/A

Related Github Issue(s)

yantosca added 2 commits July 31, 2023 18:06
@yantosca
Updates to netCDF modules in the NcdfUtil folder
e492d8c 
NcdfUtil/m_netcdf_io_define.F90
- In routine NF90_Set_Fill, use ifill argument instead of the
  parameter NF90_NOFILL in the call to NF90_Set_Fill.  This error
  turning off filling regardless of what the value of ifill was.

NcdfUtil/m_netcdf_io_write.F90
- Fixed incorrect subroutine header comment

NcdfUtil/ncdf_mod.F90
- Remove USE statements at top of module.  Instead include USE statements
  within each subroutine. This will make all imported routines
  PRIVATE to ncdf_mod.F90.
- Removed leftover "netcdf.inc" from the netCDF-F77 interface
- Use NF90_UNLIMITED instead of NF_UNLIMITED (which is netCDF-F77)
- Remove unnecessary ELSE branches
- Whitespace updates, trimmed trailing whitespace
- Remove NC_READ_VAR_CORE.  Add its functionality into overloaded
  module routines NC_READ_VAR_SP and NC_READ_VAR_DP.

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca
Bug fix: Use singleton dims in arrays passed to netCDF write routines
cd3b7de 
History/history_netcdf_mod.F90
- Remove leftover "netcdf.inc" from the netCDF-F77 interface
- Use NF90_UNLIMITED instead of NF_UNLIMITED
- Allocate NcData_* arrays with a singleton extra dimension for time
  before passing to the NcWr routine.  This avoids the corrupted
  data issue described by @nicholasbalasus in #1888.

Signed-off-by: Bob Yantosca <[email protected]>
@yantosca yantosca added category: Bug Something isn't working topic: Diagnostics Related to output diagnostic data labels Aug 1, 2023
@yantosca yantosca requested a review from msulprizio August 1, 2023 15:20
@yantosca yantosca self-assigned this Aug 1, 2023
@yantosca
Copy link
Contributor Author

yantosca commented Aug 1, 2023

@msulprizio: I didn't add a changelog update since this is fixing a bug in an update that was introduced in 14.2.1.

@yantosca
Copy link
Contributor Author

yantosca commented Aug 1, 2023

Also note: This raises the need for an integration test with instantaneous diagnostic output.

@yantosca
Copy link
Contributor Author

yantosca commented Aug 1, 2023

Integration tests are running.

@yantosca yantosca linked an issue Aug 1, 2023 that may be closed by this pull request
14.2.1 nested grid CH4 simulation doesn't write hourly SpeciesConcVV_CH4 properly #1888
Closed
4 tasks
@yantosca
Copy link
Contributor Author

yantosca commented Aug 1, 2023

After merging atop PR #1891, all GCHP integration tests passed:

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

GCClassic #2bf8d29 Merge PR #1891 (Fix incorrect time slice for boundary conditions)
GEOS-Chem #42159b1e8 Merge PR #1896 (Pass arrays w/ singleton time dim to netCDF routines)
HEMCO     #477c7e8 Merge PR #229 (Fix incorrect longitude definitions for HEMCO standalone)

Number of execution tests: 5

Submitted as SLURM job: 64882169
==============================================================================
 
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 integration tests were zero-diff w/r/t PR #1891

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
Copy link
Contributor Author

yantosca commented Aug 1, 2023

After merging on top of PR #1891, all integration tests passed (except for TOMAS, which is a known issue):

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

GCClassic #2d16a31 Merge PR #1891 (Fix incorrect time slice for boundary conditions)
GEOS-Chem #42159b1e8 Merge PR #1896 (Pass arrays w/ singleton time dim to netCDF routines)
HEMCO     #477c7e8 Merge PR #229 (Fix incorrect longitude definitions for HEMCO standalone)

Using 24 OpenMP threads
Number of execution tests: 26

Submitted as SLURM job: 64881964
==============================================================================
 
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, all integration tests were zero-diff w/r/t PR #1891, except for:

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

GCClassic #2d16a31 Merge PR #1891 (Fix incorrect time slice for boundary conditions)
GEOS-Chem #42159b1e8 Merge PR #1896 (Pass arrays w/ singleton time dim to netCDF routines)
HEMCO     #477c7e8 Merge PR #229 (Fix incorrect longitude definitions for HEMCO standalone)

Using 24 OpenMP threads
Number of execution tests: 26

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

msulprizio
msulprizio approved these changes Aug 1, 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.

Looks good to merge

@yantosca yantosca merged commit 42159b1 into dev/14.2.1 Aug 1, 2023
@yantosca yantosca deleted the bugfix/hourly-diagnostics branch August 1, 2023 18:34
@yantosca yantosca mentioned this pull request Aug 1, 2023
Closed
4 tasks
@yantosca yantosca added category: Bug Fix Fixes a previously-reported bug and removed category: Bug Something isn't working labels Aug 1, 2023
@yantosca yantosca mentioned this pull request Aug 1, 2023
Merged
1 task
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: Bug Fix Fixes a previously-reported bug topic: Diagnostics Related to output diagnostic data
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

14.2.1 nested grid CH4 simulation doesn't write hourly SpeciesConcVV_CH4 properly
2 participants
@yantosca @msulprizio