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

Update NUOPC/CMEPS driver #607

Merged
merged 133 commits into from
Jun 17, 2021

Conversation

DeniseWorthen
Copy link
Contributor

@DeniseWorthen DeniseWorthen commented Jun 11, 2021

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

  • Short (1 sentence) summary of your PR:
    Updates drivers/nuopc/cmeps for both CESM and UFS
  • Developer(s):
    @DeniseWorthen
    @dabail10
    @mvertens
  • Suggest PR reviewers from list in the column to the right.
    @dabail10
    @apcraig
  • Please copy the PR test results link or provide a summary of testing completed below.
    Regression test logs for Cheyenne Intel and GNU for ufs-weather against current develop branch
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:
  • Code changes are confined to drivers/nuopc/cmeps with the exceptions of making makemask public in
    cicecore/cicedynB/infrastructure/ice_grid.F90 and minor changes need in ice_init for CESM

  • adds drivers/nuopc/cmeps updates from ESCOMP including introduction of mask file for generating mask when appropriate and changes that allow cice6 to run as an external cesm component

  • makes changes in drivers/nuopc/cmeps required for new time manager and introduces a run configurable mesh tolerance error for ufs-weather

  • removes forapps/ufs subdirectory which is no longer required by ufs-weather

These changes are B4B when tested against the current develop branch of ufs-weather-model (00d570e). Tested configurations for ufs-weather-model include multiple configurations, e.g. a fully active atmosphere (UFSAtm) or a data atmosphere (CDEPS, NEMSDatm) coupled to MOM6 or MOM6+WW3 using the CMEPS mediator. Tests are conducted primarily for different ATM physics options and restart reproducibility of the coupled system. Tested resolutions are C96 ATM
and 1deg OCN+ICE, C192 ATM and 1/2deg OCN+ICE and C384 ATM and 1/4 deg OCN+ICE. Both Intel and GNU compilers are tested for the coupled system.

DeniseWorthen and others added 30 commits February 25, 2020 08:43
* Isotopes for CICE (CICE-Consortium#423)

Co-authored-by: apcraig <anthony.p.craig@gmail.com>
Co-authored-by: David Bailey <dbailey@ucar.edu>
Co-authored-by: Elizabeth Hunke <eclare@lanl.gov>
Update CICE for coupling with UFS
changes to satisfy ufsatm and cesm requirements for pot temp and density from atm
* Add atmiter_conv to CICE

* Add documentation

* trigger build the docs

Co-authored-by: David A. Bailey <dbailey@ucar.edu>
@@ -452,6 +450,8 @@ subroutine input_data
#ifndef CESMCOUPLED
runid = 'unknown' ! run ID used in CESM and for machine 'bering'
runtype = 'initial' ! run type: 'initial', 'continue'
restart = .false. ! if true, read restart files for initialization
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is needed because CESM has restart flags set by driver.

@@ -1775,7 +1775,7 @@ subroutine input_data
grid_type /= 'rectangular' .and. &
grid_type /= 'cpom_grid' .and. &
grid_type /= 'regional' .and. &
grid_type /= 'latlon' ) then
grid_type /= 'setmask' ) then
Copy link
Contributor

Choose a reason for hiding this comment

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

The latlon grid functionality has changed here. We are using a new NUOPC mesh functionality. Are there other groups that use the 'latlon' grid?

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to keep the latlon capability. F cases use the latlon feature in prescribed ice mode, I assume there may be other users of older version of CESM that need this. RASM certainly does.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't realize that RASM still did F compsets! We can certainly keep this. I can make this modification.

@@ -40,7 +40,7 @@ module ice_grid
private
public :: init_grid1, init_grid2, &
t2ugrid_vector, u2tgrid_vector, &
to_ugrid, to_tgrid, alloc_grid
to_ugrid, to_tgrid, alloc_grid, makemask
Copy link
Contributor

Choose a reason for hiding this comment

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

makemask is needed in the driver code

@@ -379,11 +379,6 @@ subroutine init_grid2
else
call popgrid ! read POP grid lengths directly
endif
#ifdef CESMCOUPLED
elseif (trim(grid_type) == 'latlon') then
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, is anyone else using latlongrid?

Copy link
Contributor

@dabail10 dabail10 left a comment

Choose a reason for hiding this comment

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

There are only two non-driver routines impacted here: ice_init.F90 and ice_grid.F90. I have added comments as a part of the review here. Otherwise, everything else is needed for running the NOAA/EMC UFS app and the CESM.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

I think this can be merged after the lonlat mode is recovered.

@dabail10
Copy link
Contributor

@DeniseWorthen can you do this? I'm not sure I can modify your pull request.

@phil-blain
Copy link
Member

I think this can be merged after the lonlat mode is recovered.

It would be nice however if the merge message would be tweaked to not include the concatenated messages from the 131(!) commits in this PR (which is the default for squash merges)...

Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes to ice_init.F90 and ice_grid.F90. Someone else should review the nuopc mods.

*  advance_timestep calls calendar; calling calendar
a second time results in hour always being equal to
hourp and new_hour is always false and no hourly
history is produced
@DeniseWorthen
Copy link
Contributor Author

@apcraig With further testing, I think I'm seeing the same behaviour as reported in issue 589. Our use case is with the following:

    histfreq       = 'm','d','h','x','x'
    histfreq_n     =  0 , 0 , 6 , 1 , 1
    hist_avg       = .true.

However, I am getting hourly history files instead of the expected 6-hourly files. It seems elapsed_hours is 0 (until the start of a new day) so that mod(elapsed_hours,histfreq_n(ns)) is always 0, and write_history is true for each new_hour.

@apcraig
Copy link
Contributor

apcraig commented Jun 16, 2021

@DeniseWorthen, sorry about the hourly output issue. This is something I could/should have fixed by now, but I thought we'd quickly settle questions about relative vs absolute frequency. I will fix this soon. If this is critical for you, let me know and I'll make it even higher priority. Thanks!

@apcraig
Copy link
Contributor

apcraig commented Jun 16, 2021

Just a final checkin @DeniseWorthen, is this ready to merge from your side of things?

@DeniseWorthen
Copy link
Contributor Author

@apcraig We would need to get it fixed prior to the next evaluation we're preparing for the coupled model, but it will be a few weeks before we're ready to run the entire set. Until then, we can make do w/ hourly output. I think it is ready to merge. Thanks.

@apcraig apcraig merged commit ff88891 into CICE-Consortium:master Jun 17, 2021
@DeniseWorthen DeniseWorthen deleted the feature/updcap_escomp_scol branch November 26, 2021 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants