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 sw_redist option to CICE #497

Merged
merged 1 commit into from Jul 31, 2020
Merged

Conversation

dabail10
Copy link
Contributor

@dabail10 dabail10 commented Jul 31, 2020

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:
    This integrates the sw_redist changes from Icepack into CICE.
  • Developer(s):
    dabail10 (D. Bailey)
  • 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.
    https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_mach_forks#cheyenne
  • 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:
    The alt03 and alt04 test cases have been updated so that ktherm=1 is bfb. The default settings are such that ktherm=2 is bfb. This is the other piece to addressing issue shortwave redistribution #485 and shortwave modification for mushy thermo layers melting completely Icepack#280.

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.

It looks like Icepack will be updated with this PR. Is that intentional?

@dabail10
Copy link
Contributor Author

Yes! This change was already merged in Icepack.

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.

Looks good. This CICE PR also updates other changes to Icepack in addition to sw redistribution.

@apcraig
Copy link
Contributor

apcraig commented Jul 31, 2020

The changes in CICE here are related to turning on the new options in Icepack. There are 3 new namelist for CICE and these are basically passed to Icepack. The changes in the alt options preserve the current CICE testing where ktherm=1 sets sw_redist=.true.. That is consistent with the bl99 having this feature hardwired before and mushy not. As we move forward, we probably want to add a test where mushy turns on the sw_redist feature and bl99 does not to be more complete.

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 am testing this now on gordon just to make sure it's all bit-for-bit (as we expect). I will merge once that test is complete.

Copy link
Contributor

@proteanplanet proteanplanet left a comment

Choose a reason for hiding this comment

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

Approved!

@apcraig
Copy link
Contributor

apcraig commented Jul 31, 2020

90% of the tests are completed on gordon and everything that has run has passed, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#91acad70660303035754f32c36eca863f00b29d1. A few are sitting in the queue, but I'm fairly confident it's all fine. I will merge this now and then update the test results when they have completed.

@apcraig apcraig merged commit 003aae0 into CICE-Consortium:master Jul 31, 2020
@dabail10 dabail10 deleted the swredist branch March 24, 2021 16:45
@phil-blain
Copy link
Member

For easier future lookups (it took me a while), here is the corresponding Icepack PR: CICE-Consortium/Icepack#326

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.

None yet

5 participants