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

Extract non-initialization parts of shr_pio_mod to a module in share #306

Merged
merged 3 commits into from
Jul 8, 2022
Merged

Extract non-initialization parts of shr_pio_mod to a module in share #306

merged 3 commits into from
Jul 8, 2022

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Jul 5, 2022 •
edited
Loading

Description of changes

Extract the non-initialization parts of shr_pio_mod to a module in the
share repository, just keeping the initialization parts here.

This is part of a set of changes where I am splitting shr_pio_mod into two pieces:

(1) Reading configuration files and initializing PIO appropriately

(2) Storing information about PIO (io system descriptors, io types, io formats) and providing an interface to query this information

Piece (2) lives in the share code and is used regardless of the driver. Piece (1) is driver-specific, so for now we have three versions of that piece: one in CMEPS (created by extracting the initialization pieces from cmeps/cesm/nuopc_cap_share/shr_pio_mod.F90), one in the cpl7 repo (created by extracting the initialization pieces from share/src/shr_pio_mod.F90), and one in CTSM's LILAC directory (which is essentially identical to the one in the cpl7 repo). Piece (2) – the actual share code piece – is used by components (their use statements stay exactly as they are now) as well as by piece (1) (which is responsible for setting the module-level variables in piece (2)).

See ESCOMP/CTSM#1759 (comment) for more context.

Needs to be coordinated with ESCOMP/CESM_share#34

Specific notes

Contributors other than yourself, if any: Discussions with @jedwards4b @mvertens

CMEPS Issues Fixed (include github issue #): none

Are changes expected to change answers? no

Any User Interface Changes (namelist or namelist defaults changes)? no

Testing performed

Only limited testing performed so far; I plan to run CESM prealpha testing. Please let me know if you'd like more than that (I'm uncertain about whether scripts_regression_tests and testlist_drv give additional value if I'm already running prealpha testing).

Testing performed if application target is CESM:

  • (recommended) CIME_DRIVER=nuopc scripts_regression_tests.py
    • machines:
    • details (e.g. failed tests):
  • (recommended) CESM testlist_drv.xml
    • machines and compilers:
    • details (e.g. failed tests):
  • (optional) CESM prealpha test
    • machines and compilers: cheyenne intel & gnu
    • details (e.g. failed tests): tests pass and are bit-for-bit
  • (other) please described in detail: the following tests pass
ERP_D_Ld10_P36x2_Vmct.f10_f10_mg37.IHistClm51BgcCrop.cheyenne_intel.clm-ciso_decStart
ERP_D_P36x2_Ld3.f10_f10_mg37.I1850Clm50BgcCrop.cheyenne_intel.clm-default
LILACSMOKE_D_Ld2.f10_f10_mg37.I2000Ctsm50NwpSpAsRs.cheyenne_intel.clm-lilac

Testing performed if application target is UFS-coupled:

  • (recommended) UFS-coupled testing
    • description:
    • details (e.g. failed tests):

Testing performed if application target is UFS-HAFS:

  • (recommended) UFS-HAFS testing
    • description:
    • details (e.g. failed tests):

Hashes used for testing:

  • CESM prealpha tests:
  • For the other tests described above:
  • UFS-coupled, then umbrella repostiory to check out and associated hash:
    • repository to check out:
    • branch/hash:
  • UFS-HAFS, then umbrella repostiory to check out and associated hash:
    • repository to check out:
    • branch/hash:

billsacks added 2 commits July 5, 2022 10:58
@billsacks
Extract non-initialization parts of shr_pio_mod to a module in share
28bcf74 
Extract the non-initialization parts of shr_pio_mod to a module in the
share repository, just keeping the initialization parts here.

Needs to be coordinated with a branch in the CESM_share repository.
@billsacks
Make pio_async_interface a module-level variable
03ce9b7 
This will be needed for ESCOMP#305, where
this variable is now referenced from another subroutine as well.
@billsacks billsacks requested a review from jedwards4b July 5, 2022 21:13
billsacks
billsacks commented Jul 5, 2022
View reviewed changes
cesm/nuopc_cap_share/init_pio_mod.F90 Outdated
end subroutine shr_pio_getiotypefromname

!===============================================================================
subroutine shr_pio_namelist_set(npes,mycomm, pio_stride, pio_root, pio_numiotasks, &
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed shr_pio_namelist_set: this appears to be unused and was buggy (in particular, it referenced the old module-level scalar pio_async_interface, but that variable was never set; now I have removed that scalar and replaced it with an array, as I describe in a separate comment).

cesm/nuopc_cap_share/init_pio_mod.F90 Outdated
integer :: pio_debug_level=0, pio_blocksize=0
integer(kind=pio_offset_kind) :: pio_buffer_size_limit=-1

type(pio_rearr_opt_t) :: pio_rearr_opts
logical, allocatable :: pio_async_interface(:)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to keep pio_comp_t the same for all drivers, I removed pio_async_interface from that derived type, and made that its own variable specific to the nuopc version (the mct version uses a scalar, the nuopc version has per-component values).

This change will intersect with #305; I will make a separate comment about what needs to be done to reconcile these PRs.

Alternatively, I could put pio_async_interface in the shared derived type and just leave this unset by the non-nuopc versions. (I made it local because it looked like it wouldn't be needed after initialization, but if it will be needed after initialization, then this would be a better approach.)

@billsacks
Copy link
Member Author

I wanted to move init_pio_mod out of nuopc_cap_share into the driver source directory (signaling that it is only used by the driver, not by other components), but the sticking point was the call to shr_pio_log_comp_settings from set_component_logging in nuopc_shr_methods.F90 (which is in the share directory). It looks like this move of the file may be possible after the changes in #305 - where I see that @jedwards4b has removed at least one call (and maybe the only call) to shr_pio_log_comp_settings from nuopc_shr_methods.F90.

I have carefully reviewed the overlapping changes in #305 . Jim's changes are just in three subroutines: shr_pio_component_init, shr_pio_log_comp_settings and shr_pio_finalize. I think that PR can be merged with this one as follows:

  1. Start with my new module (i.e., from this PR)
  2. Delete the above three routines and replace them with Jim's new versions
  3. Rename these three routines (and references to them) to match the names in this PR
  4. In these three modified routines from first step - reorder pio_init and move to ensemble_driver #305, change references to pio_async_interface to refer to the new module-level variable in this PR rather than an element of the derived type.
  5. Also be sure to allocate pio_async_interface in init_pio_component_init as is done in this PR.

I would be happy to help with this merge.

@billsacks billsacks mentioned this pull request Jul 5, 2022
Merged
@billsacks
Copy link
Member Author

I have run CESM prealpha testing on these changes. All tests pass other than those that failed in the baseline as well. I have updated the top-level comment with details. @jedwards4b let me know if you'd like me to do any other testing – and if you'd like me to do the srt testing by hand, please help me with exactly how to do that.

@billsacks billsacks merged commit 3782c38 into ESCOMP:master Jul 8, 2022
@billsacks billsacks deleted the fix_lilac_pio2 branch July 8, 2022 17:49
billsacks added a commit to ESCOMP/CESM_CPL7andDataComps that referenced this pull request Jul 8, 2022
@billsacks
Merge pull request #16 from billsacks/fix_lilac_pio2
ce5b9d9 
Introduce init_pio_mod with the initialization pieces from shr_pio_mod

This PR introduces an init_pio_mod that contains just the initialization pieces from shr_pio_mod (from the share repository).

This is part of a set of changes where I am splitting shr_pio_mod into two pieces: 

(1) Reading configuration files and initializing PIO appropriately

(2) Storing information about PIO (io system descriptors, io types, io formats) and providing an interface to query this information

Piece (2) lives in the share code and is used regardless of the driver. Piece (1) is driver-specific, so for now we have three versions of that piece: one in CMEPS (created by extracting the initialization pieces from `cmeps/cesm/nuopc_cap_share/shr_pio_mod.F90`), one in the cpl7 repo (created by extracting the initialization pieces from `share/src/shr_pio_mod.F90`), and one in CTSM's LILAC directory (which is essentially identical to the one in the cpl7 repo). Piece (2) – the actual share code piece – is used by components (their use statements stay exactly as they are now) as well as by piece (1) (which is responsible for setting the module-level variables in piece (2)).

See ESCOMP/CTSM#1759 (comment) for more context.

Needs to be coordinated with ESCOMP/CESM_share#34 . See also the related PR for CMEPS: ESCOMP/CMEPS#306
billsacks added a commit to ESCOMP/CTSM that referenced this pull request Jul 11, 2022
@billsacks
Merge branch 'fix_lilac_pio2'
1e380f8 
Fix LILAC interface to PIO

Fixes the LILAC interface to PIO.

This involved splitting shr_pio_mod into two pieces:

(1) Reading configuration files and initializing PIO appropriately

(2) Storing information about PIO (io system descriptors, io types, io
formats) and providing an interface to query this information

Piece (2) lives in the share code and is used regardless of the driver.
Piece (1) is driver-specific, so for now we have three versions of that
piece: one in CMEPS (created by extracting the initialization pieces
from cmeps/cesm/nuopc_cap_share/shr_pio_mod.F90), one in the cpl7 repo
(created by extracting the initialization pieces from
share/src/shr_pio_mod.F90), and one in CTSM's LILAC directory (which is
essentially identical to the one in the cpl7 repo). Piece (2) – the
actual share code piece – is used by components (their use statements
stay exactly as they are now) as well as by piece (1) (which is
responsible for setting the module-level variables in piece (2)). See
#1759 (comment) for
more context.

Much of the work here was in externals:
ESCOMP/CESM_share#34,
ESCOMP/CMEPS#306 and
ESCOMP/CESM_CPL7andDataComps#16. So this PR
updates those externals to versions with those changes. In addition,
changes were needed within LILAC - to implement the LILAC version of
piece (1) described above.

Resolves #1759 (LILAC test failing in ctsm5.1.dev095 with
cime/share/pio update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedwards4b jedwards4b jedwards4b approved these changes

Assignees

@jedwards4b jedwards4b

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@billsacks @jedwards4b