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 mct/cesm1 driver, fix ss_tlt implementation #726

Merged
merged 2 commits into from
Jun 3, 2022

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Jun 2, 2022

PR checklist

  • Short (1 sentence) summary of your PR:
    Update mct/cesm1 driver, fix ss_tlt implementation
  • Developer(s):
    apcraig, deniseworthen, dabail10
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Bit-for-bit in CICE standalone, full test suites run on cheyenne and narwhal. https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#74632f4de904bf890f2a490e5ea3df17f799081a
  • 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:

Update ss_tltx, ss_tlty averaging/usage in evp, eap, vp

  • this should change answers but does not in standalone CICE because ss_tlt is always 0 in testing
  • it does impact coupled runs
    Update mct/cesm1 driver for snow mods and other recent changes
    Remove USE_NETCDF cpp in io_pio2/ice_restart.F90, not needed or correct

Update mct/cesm1 driver for snow mods and other recent changes
Remove USE_NETCDF cpp in io_pio2/ice_restart.F90, not needed or correct
limited situations where we want to ensure bit-for-bit across the seam,
but do NOT want to update the halo in general.  In particular, this is
needed for UAREA during initialization.  UAREA is explicitly computed
in the halo to avoid missing data.  In some cases, uarea computed
independently along the seam in the halo points results in roundoff
level differences.  This will produce a small error in the simulation
but more important, breaks the symmetry across the seam.  In that case,
we just want to sync up the halo, but we do NOT want to update the
independent UAREA halo computations elsewhere because of land block
elimination and other situations where blocks have no neighbors.
Copy link
Contributor

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

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

I understand the changes. They look reasonable. I'll approve in advance.

@apcraig
Copy link
Contributor Author

apcraig commented Jun 3, 2022

I reran the full test suite on cheyenne after adding the tripoleOnly option to the haloupdate call. Everything is still bit-for-bit. I also tested the code in UFS, which highlighted the issue and this seems to fix a problem we saw there.

https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#c3c9ce1d4b5e780eef35eb85992ffaf06d0d1f5c

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.

Glad you've found a way to make this work.

@apcraig apcraig merged commit 7705e13 into CICE-Consortium:main Jun 3, 2022
@apcraig apcraig mentioned this pull request Jun 3, 2022
16 tasks
dabail10 pushed a commit to ESCOMP/CICE that referenced this pull request Oct 4, 2022
* Update ss_tltx, ss_tlty averaging/usage in evp, eap, vp
Update mct/cesm1 driver for snow mods and other recent changes
Remove USE_NETCDF cpp in io_pio2/ice_restart.F90, not needed or correct

* Add tripoleOnly flag to ice_HaloUpdate2DR8.  This is needed in very
limited situations where we want to ensure bit-for-bit across the seam,
but do NOT want to update the halo in general.  In particular, this is
needed for UAREA during initialization.  UAREA is explicitly computed
in the halo to avoid missing data.  In some cases, uarea computed
independently along the seam in the halo points results in roundoff
level differences.  This will produce a small error in the simulation
but more important, breaks the symmetry across the seam.  In that case,
we just want to sync up the halo, but we do NOT want to update the
independent UAREA halo computations elsewhere because of land block
elimination and other situations where blocks have no neighbors.
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.

3 participants