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

Add nuopc shr methods #269

Closed
wants to merge 17 commits into from
Closed

Conversation

jedwards4b
Copy link
Contributor

Description of changes

move nuopc_shr_methods.F90 from cmeps/cesm/nuopc_cap_share

Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial):

Any User Interface Changes (namelist or namelist defaults changes):

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):

Hashes used for testing:

mvertens and others added 11 commits November 16, 2021 11:02
cmeps directory refactor
### Description of changes
CMEPS directory refactor
### Specific notes
The following directory changes were made:
- util/ => ufs/
- nuopc_cap_share => cesm/nuopc_cap_share/
- share/share/shr_flux_mod => cesm/flux_atmocn/shr_flux_mod (cesm only)
- share/cmeps/* => cesm/nuopc_cap_share (cesm only)
Also shr_constants_mod.F90 is now only used by cesm and the the file util/shr_const_mod.F90 has been moved and renamed to ufs/ufs_constants_mod.F90
In addition med_aofluxes_mod.F90 now has separate calls to the ufs and cesm atmocn flux computation.
Contributors other than yourself, if any:
CMEPS Issues Fixed: None
Are changes expected to change answers? bit-for-bit
Any User Interface Changes (namelist or namelist defaults changes)? No
Testing performed:
CESM testing: verified that the following tests are bfb with cesm2_3_alpha07a on cheyenne :
   IRT_Ld7.f09_g17.BHIST.cheyenne_intel.allactive-defaultio
   ERS_Ld3.f45_g37_rx1.A.cheyenne_intel
UFS testing: ufs-weather-model [0cf60348](https://github.com/ufs-community/ufs-weather-model) using CMEPS e703499
### Description of changes
Update shr_pio_mod, removing mct centric initialization.
Improve logging and make sure logs go to the correct component.
Depends on cime branch async_io_in_esmf

### Specific notes

Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers? bfb

Any User Interface Changes (namelist or namelist defaults changes)?
The modelio namelist has been eliminated and the parameters from that file were moved to nuopc.runconfig,
The xml variable PIO_ASYNC_INTERFACE was changed from a global variable to one that could be set individually
for each component.

### Testing performed

Testing performed if application target is CESM:
- [X] (recommended) CIME_DRIVER=nuopc scripts_regression_tests.py
   - machines: cheyenne intel
   - details (e.g. failed tests): IRT_N2_Vmct_Ln9.f19_g16_rx1.A.cheyenne_intel fails, apparently expected.

- [ ] (recommended) CESM testlist_drv.xml
   - machines and compilers:
   - details (e.g. failed tests):
- [x] (optional) CESM prealpha test
   - machines and compilers
   - details (e.g. failed tests):
- [ ] (other) please described in detail
   - machines and compilers
   - details (e.g. failed tests):

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:
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch/hash:
- [ ] 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:
Extract non-initialization parts of shr_pio_mod to a module in share

### 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/share/shr_pio_mod.F90`), one in the cpl7 repo (created by extracting the initialization pieces from `share/share/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):
- [x] (optional) CESM prealpha test
   - machines and compilers: cheyenne intel & gnu
   - details (e.g. failed tests): tests pass and are bit-for-bit
- [x] (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:

- [x] CESM prealpha tests:
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch/hash: cesm2_3_alpha09c, but with: cmeps at 1f8ce13, cpl7 at 15c5d5ce45a9db320b1448e5c29d9892fc57e046 and share at d7c43983b8d84abfc357fa112870bc50b3b60d60 (all from billsacks forks)
- [x] For the other tests described above:
  - repository to check out: https://github.com/billsacks/CTSM.git
  - branch/hash: branch fix_lilac_pio2, hash 2ead6826d
- [ ] 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:
first step - reorder pio_init and move to ensemble_driver
### Description of changes
Add an InitializeIO phase to the ensemble_driver, this allows ESMF to control the ASYNCIO tasks internally.

### Specific notes
It requires however that components do not do IO initialization until the realize phase so the cice and mosart component PRs: ESCOMP/MOSART#55
ESCOMP/CICE#18 must be merged first.
Revert "first step - reorder pio_init and move to ensemble_driver"
### Description of changes
Make the med_fldList fields a linked list instead of an array.   This avoids a lot of small allocations and memory fragmentation.

### Specific notes

Contributors other than yourself, if any: Denise

CMEPS Issues Fixed: #321

Are changes expected to change answers? bfb

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

### Testing performed

Testing performed if application target is CESM:
- [X] (recommended) CIME_DRIVER=nuopc scripts_regression_tests.py
   - machines: cheyenne, intel
   - details (e.g. failed tests):  all pass
- [ ] (recommended) CESM testlist_drv.xml
   - machines and compilers:
   - details (e.g. failed tests):
- [X] (optional) CESM prealpha test
   - machines and compilers cheyenne, intel
   - details (e.g. failed tests):  All pass except expected fails of cesm2_3_alpha10c
- [ ] (other) please described in detail
   - machines and compilers
   - details (e.g. failed tests):

Testing performed if application target is UFS-coupled:
- [X] (recommended) UFS-coupled testing
   - description: gnu and intel
   - 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:
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch/hash:
- [ ] 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:
move shr_file_getLogUnit to shr_log_getLogUnit
### Description of changes
Clean up usage of log files

### Specific notes
Depricated shr_file_getLogUnit and added shr_log_getLogUnit
Depends on ESCOMP/CESM_share#36
Contributors other than yourself, if any:

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)

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

### Testing performed

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):
- [X] (optional) CESM prealpha test
   - machines and compilers cheyenne intel
   - details (e.g. failed tests): all consistant with baselines.
- [ ] (other) please described in detail
   - machines and compilers
   - details (e.g. failed tests):

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:
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch/hash:
- [ ] 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:
enable asyncio using pio
### Description of changes
Allows IO tasks to be independent of compute tasks in cesm

### Specific notes
(testing in progress)
Contributors other than yourself, if any:
Depends on share (ESCOMP/CESM_share#37) and cime (ESMCI/cime#4340).

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)

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

### Testing performed

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):
- [X] (optional) CESM prealpha test
   - machines and compilers cheyenne intel
   - details (e.g. failed tests): results consistant with cesm2_3_alpha10d
- [ ] (other) please described in detail
   - machines and compilers
   - details (e.g. failed tests):

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:
- [X] (recommended) UFS-HAFS testing
   - description:
   - details (e.g. failed tests):

### Hashes used for testing:

- [ ] CESM:
  - repository to check out: https://github.com/ESCOMP/CESM.git
  - branch/hash:
- [ ] 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:
@jedwards4b jedwards4b requested a review from uturuncoglu April 1, 2024 17:57
@jedwards4b
Copy link
Contributor Author

This moves the file nuopc_shr_methods.F90 from cmeps/cesm/nuopc_cap_share to cdeps/share with history so that it can be used by ufs. Note that the functionality here is not supported by ufs and so an error will be generated, but it will compile.

@uturuncoglu
Copy link
Collaborator

@jedwards4b Do you want me to test this with UFS? or wait you to fix any outstanding issue.

@jedwards4b
Copy link
Contributor Author

You can test this with UFS - I am working on an issue with REST_OPTION=end but that can be separate from this initial test of functionality.

Copy link
Collaborator

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

It looks good to me. I am now running some tests in UFS side and update you soon.

@uturuncoglu
Copy link
Collaborator

@jedwards4b I run couple of data component test with UFS Weather Model develop and all pass without any issue. Here is the list of tests:

datm_cdeps_mx025_cfsr - intel
datm_cdeps_lnd_era5_rst - intel
datm_cdeps_restart_cfsr - intel

I think this PR is fine for UFS.

Copy link
Collaborator

@uturuncoglu uturuncoglu left a comment

Choose a reason for hiding this comment

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

Tests look fine in UFS side.

@mvertens
Copy link
Collaborator

@jedwards4b - is this ready to be merged?

@jedwards4b
Copy link
Contributor Author

@mvertens I think that there is still an issue with building it for cesm and I wasn't planning to bring it in until after beta17.

@mvertens
Copy link
Collaborator

@jedwards4b - thanks for the update.

@jedwards4b
Copy link
Contributor Author

@uturuncoglu I would prefer to remove the share code from cdeps completely and require ufs to use https://github.com/ESCOMP/CESM_share. If including another repository is an issue then maybe a fork or ufs branch could include it without it being in the cesm development tree.

@uturuncoglu
Copy link
Collaborator

@jedwards4b It is fine for me but I am not sure about what UFS developer team think. So, it would be nice to inform them about this. @junwang-noaa @DeniseWorthen maybe we could discuss it in the next infrastructure meeting. Let me know what you think.

@NickSzapiro-NOAA
Copy link

Hi. I'm a code manager for https://github.com/NOAA-EMC/CDEPS/ for UFS. Adding a submodule is not without overhead. It would be very nice to discuss this further, especially if there are shared benefits to these changes. If I may ask, does CESM_share introduce any additional dependencies/requirements?

@jedwards4b
Copy link
Contributor Author

@NickSzapiro-NOAA No CESM_share does not add any additional dependencies. It did in the past and that I think is why we needed to make a copy of it without them, but those have been removed. I don't like having multiple copies of the same source around and so now that ufs cdeps can use the cesm_share repo instead of having it's own copy I would really like to delete the copy.

@DeniseWorthen
Copy link
Collaborator

In ufs, I see the use of nuopc_shr_methods in CMEPS-interface/CMEPS/cesm as well as in the following locations:

CDEPS-interface/CDEPS/dshr/dshr_mod.F90:117:    use nuopc_shr_methods, only : set_component_logging
CMEPS-interface/CMEPS/mediator/med.F90:568:    use nuopc_shr_methods, only : set_component_logging
MOM6-interface/MOM6/config_src/drivers/nuopc_cap/mom_cap.F90:36:use nuopc_shr_methods, only: get_component_instance

In each of these cases, both the use nuopc_shr_methods as well as the associated calls are w/in a CESMCOUPLED ifdef block.

The removal of the nuopc_shr_methods from CMEPS/cesm should have no impact on UFS.

As far as removing all of CDEPS/share from CDEPS itself, we do use that code so I think that needs a larger discussion.

@jedwards4b
Copy link
Contributor Author

@DeniseWorthen The code is in https://github.com/ESCOMP/CESM_share
there is no reason to duplicate it in cdeps and duplicating code is error prone.

@DeniseWorthen
Copy link
Collaborator

I understand why you want to do it, but for UFS it requires adding another submodule and maintaining it.

@jedwards4b
Copy link
Contributor Author

You could alternatively create a fork of cdeps with these files and maintain that. The way it is now is requiring me to maintain two repositories of these shared files.

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Jun 27, 2024

We already maintain a fork of CDEPS. I can see that moving these files gives you one less thing to manage. We'd then be on the hook for ensuring that we keep up to date w/ any changes that might come in to CESM_share. I see a lot of commits to CESM_share, but it's unclear to me how much is actually changing w/ those commits.

Also, I see in CESM_share some of the F90.in files that required the use of the FOX library (?), which we couldn't do because of operational platforms. Isn't that going to be a conflict if we were to use CESM_share? (I also don't know if CESM_share has other dependencies like FOX which would be a problem.)

@jedwards4b
Copy link
Contributor Author

The .F90.in files require the use of genf90.pl something I think you already use.

@DeniseWorthen
Copy link
Collaborator

We actually use that outside of the repo and update the files manually. We can't use genf90.pl directly because of operational restrictions.

@jedwards4b
Copy link
Contributor Author

Most of those recent commits in CESM_share were debugging the github workflow and didn't actually change any source. It sounds like you should consider maintaining a copy of these files on your fork.

@jedwards4b
Copy link
Contributor Author

Added in commit commit e3aeb54
Author: Jim Edwards jedwards@ucar.edu
Date: Wed Aug 21 14:21:59 2024 -0600

add nuopc_shr_methods.F90 for UFS

@jedwards4b jedwards4b closed this Sep 6, 2024
@jedwards4b jedwards4b deleted the add_nuopc_shr_methods branch September 6, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants