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

Remove requirement for photolysis data directories to end with slash #2225

Merged

Conversation

lizziel
Copy link
Contributor

@lizziel lizziel commented Mar 27, 2024

Name and Institution (Required)

Name: Lizzie Lundgren
Institution: Harvard University

Describe the update

This PR removes the expectation that data directories used for lookup tables read in photolysis_mod.F90 gave a trailing slash. This gives greater flexibility with defining these paths externally to the model.

Expected changes

This is a no diff update.

Reference(s)

None

Related Github Issue(s)

None

@lizziel lizziel changed the base branch from main to dev/no-diff-to-benchmark March 27, 2024 21:28
@lizziel lizziel added category: Feature Request New feature or request no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations labels Mar 27, 2024
@lizziel lizziel added this to the 14.3.1 milestone Mar 27, 2024
… path

Signed-off-by: Lizzie Lundgren <elundgren@seas.harvard.edu>
@lizziel lizziel force-pushed the feature/do_not_require_data_dir_end_slash branch from 2d6f4ff to df50721 Compare March 27, 2024 21:30
@yantosca
Copy link
Contributor

yantosca commented Mar 28, 2024

@lizziel: I just remembered we have this routine in GeosCore/input_mod.F90:

!EOC
!------------------------------------------------------------------------------
!                  GEOS-Chem Global Chemical Transport Model                  !
!------------------------------------------------------------------------------
!BOP
!
! !IROUTINE: check_directory
!
! !DESCRIPTION: Makes sure that the given directory is valid.  Also a trailing
!  slash character will be added if necessary.
!\\
!\\
! !INTERFACE:
!
  SUBROUTINE Check_Directory( Input_Opt, dir, RC )
!
! !USES:
!
    USE ErrCode_Mod
    USE FILE_MOD,      ONLY : File_Exists
    USE Input_Opt_Mod, ONLY : OptInput
!
! !INPUT/OUTPUT PARAMETERS:
!
    TYPE(OptInput),   INTENT(INOUT) :: Input_Opt   ! Input Options object
    CHARACTER(LEN=*), INTENT(INOUT) :: dir         ! Dir to be checked
!
! !OUTPUT PARAMETERS:
!
    INTEGER,          INTENT(OUT)   :: RC          ! Success or failure
!
! !REVISION HISTORY:
!  20 Mar 2003 - R. Yantosca - Initial version
!  See https://github.com/geoschem/geos-chem for complete history
!EOP
!------------------------------------------------------------------------------
!BOC
!
! !LOCAL VARIABLES:
!
    ! Scalars
    INTEGER            :: C

    ! Strings
    CHARACTER(LEN=255) :: errMsg, thisLoc

    !=================================================================
    ! Check_Directory begins here!
    !=================================================================

    ! Initialize
    RC      = GC_SUCCESS
    errMsg  = ''
    thisLoc = ' -> at Check_Directory (in module GeosCore/input_mod.F90)'

    ! Locate the last non-white-space character of NEWDIR
    C = LEN_TRIM( dir )

    ! Add the trailing directory separator if it is not present
    IF ( dir(C:C) /= '/' ) THEN
       dir(C+1:C+1) = TRIM( '/' )
    ENDIF

    !=================================================================
    ! Test if the directory actually exists
    !=================================================================

    ! If the directory does not exist then stop w/ an error message
    IF ( .not. File_Exists( dir ) ) THEN
       errMsg = 'Invalid directory: ' // TRIM( dir )
       CALL GC_Error( errMsg, RC, thisLoc )
       RETURN
    ENDIF

  END SUBROUTINE Check_Directory

So we could write a function to do something like this:

    ! Locate the last non-white-space character of NEWDIR
    C = LEN_TRIM( dir )

    ! Remove the trailing directory separator if it is not present
    IF ( dir(C:C) /= '/' ) THEN
       dir = TRIM( dir(1:C-1) )
    ENDIF

@lizziel
Copy link
Contributor Author

lizziel commented Mar 28, 2024

CESM does not use input_mod.F90 to set the paths, but reads it from a namelist xml file. The convention for those files is to not include a trailing slash.

@yantosca
Copy link
Contributor

CESM does not use input_mod.F90 to set the paths, but reads it from a namelist xml file. The convention for those files is to not include a trailing slash.

Ah OK. I thought we could parse it on the GEOS-Chem side but maybe that's not possible here.

@yantosca yantosca added the topic: Structural Modifications Related to GEOS-Chem structural modifications (as opposed to scientific updates) label Mar 28, 2024
@lizziel lizziel requested a review from yantosca March 28, 2024 20:13
@lizziel lizziel marked this pull request as ready for review March 28, 2024 20:13
@yantosca yantosca self-assigned this Mar 28, 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.

This is a simple fix and I approve it. Thanks @lizziel. I will bring this into 14.3.1.

@yantosca yantosca merged commit a185885 into dev/no-diff-to-benchmark Mar 28, 2024
@yantosca
Copy link
Contributor

All GEOS-Chem Classic integration tests passed:

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

GCClassic #fc586ba GEOS-Chem submod update: Merge PR #2226 (Fix bug for ST80_25 tracer)
GEOS-Chem #a18588576 Merge PR 2225# (Don't expect photolysis dirs to end in "/")
HEMCO     #2a4bb12 Merge PR #268 (Handle 3D NEI emissions)

Using 24 OpenMP threads
Number of execution tests: 26

Submitted as SLURM job: 25619555
==============================================================================
 
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Also, all integration tests were zero-diff w/r/t 14.3.0 except:

  • APM (known parallelization issue)
  • RRTMG (numerical noise in RRTMG collection)

@yantosca
Copy link
Contributor

All GCHP integration tests passed:

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

GCHP      #cf81166 GEOS-Chem submod update: Merge PR #2226 (Fix bug for ST80_25 tracer)
GEOS-Chem #a18588576 Merge PR 2225# (Don't expect photolysis dirs to end in "/")
HEMCO     #2a4bb12 Merge PR #268 (Handle 3D NEI emissions)

Number of execution tests: 6

Submitted as SLURM job: 25621522
==============================================================================

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

Also, all GCHP integration tests were zero-diff w/r/t 14.3.0.

@msulprizio msulprizio deleted the feature/do_not_require_data_dir_end_slash branch March 29, 2024 14:25
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 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