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

icepack: optionally compute 'dragio' using under-ice roughness length #612

Merged
merged 2 commits into from
Jun 24, 2021

Conversation

phil-blain
Copy link
Member

@phil-blain phil-blain commented Jun 22, 2021

PR checklist

  • Short (1 sentence) summary of your PR:
    See title.
  • Developer(s):
    P Blain.
  • 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.
    I'm running the base_suite now and will update with the results.
    EDIT 25/06 Here are the results:
322 measured results of 322 total results
320 of 322 tests PASSED
0 of 322 tests PENDING
2 of 322 tests MISSING data
0 of 322 tests FAILED
$ ./results.csh | \grep MISS
MISS daley_intel_smoke_gx3_4x1_calcdragio complog ff88891 missing-data
MISS daley_intel_smoke_gx3_4x1_calcdragio compare ff88891  -1 -1 -1 missing-data

So everything is as expected (the missing data is for the test I'm adding in this PR).

  • 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?

  • 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 first commit just updates Icepack to the latest version, adds calc_dragio and iceruf_ocn to the CICE namelist and adds a test with ̀calc_dragio=T`. I added both variables to the index in the doc and I felt that was enough in terms of documentaiton; the Roy et al 2015 paper is already referenced in the Icepack doc.

  • The second commit is not absolutely necessary, but I think that it's more consistent to use the 1st ocean level thickness if we have access to it internally, so that's why I added that change.

In CICE-Consortium/Icepack@a80472b (icepack_parameters: optionally
compute 'dragio' from under-ice roughness (CICE-Consortium/Icepack#366),
2021-06-22), Icepack was updated to optionally compute the ice-ocean
drag coefficicent 'dragio' using an under-ice roughness length and the
thickness of the first ocean level.

Leverage this new feature in CICE by adding 'calc_dragio' and
'iceruf_ocn' to the CICE namelist. Add the new variables to the index in
the documentation and add a test with the new feature (using default
values for 'iceruf_ocn' and 'thickness_ocn_layer1').

As this new feature will mostly be useful in a coupled context, we do
not add 'thickness_ocn_layer1' to the namelist as it is expected that
the ocean model will pass this information to CICE.
In the previous commit we updated Icepack to allow computing the
ice-ocean drag coefficient 'dragio' using an under-ice roughness length
and the thickness of the first ocean layer, 'thickness_ocean_layer1', a
new Icepack parameter.

In some situations, we have access in CICE to the thicknesses of the
ocean levels, either hard-coded (use_bathymetry = false,
bathymetry_format = default), read from a file (use_bathymetry = true,
bathymetry_format = pop), or generated from the kmt_file
(use_bathymetry = false, bathymetry_format = pop). In these situations,
for consistency set 'thickness_ocean_layer1' in Icepack to the thickness
of the first ocean level, 'thick(1)' if 'calc_dragio' is active.
@phil-blain
Copy link
Member Author

cc @JFLemieux73

@apcraig
Copy link
Contributor

apcraig commented Jun 22, 2021

I think this is ready to merge?

@phil-blain, I assume this is pointing to a804 in icepack, current head of Icepack master in CICE-Consortium? Is there anyway to tell that in the PR? How do we know it's pointing to something on the CICE-Consortium master versus something on a branch in another fork?

@phil-blain
Copy link
Member Author

Yes, it is ready to merge, if you feel we don't need to wait for the base_suite results. I will add them today. We can wait for the tests.

Re Icepack, if you go to the "Files changed" tab of the PR, scroll to the bottom where Icepack is listed, and click on the "9 files" link, you see that the URL of the "Comparing changes" pages that this links to ends with 37f2a17b97a5314c2c76c7ccd30b9bada9653bd0...a80472b547aa6d7a85f8ae5e1449273a323e0371, so that means Icepack is changed from CICE-Consortium/Icepack@37f2a17 to CICE-Consortium/Icepack@a80472b, which is indeed the tip of Icepack's master at the moment.

One way to check on GtiHub if the Icepack commit this PR points to is indeed merged to the Consortium's Icepack master branch is to follow the link to the commit page: CICE-Consortium/Icepack@a80472b. At the bottom of the box with the commit message you can see "master (#366)" which means that this commit entered the history of the master branch when PR CICE-Consortium/Icepack#366 was merged.

@apcraig apcraig merged commit 995f3af into CICE-Consortium:master Jun 24, 2021
@phil-blain
Copy link
Member Author

Thanks @apcraig. The test results are as expected:

322 measured results of 322 total results
320 of 322 tests PASSED
0 of 322 tests PENDING
2 of 322 tests MISSING data
0 of 322 tests FAILED
$ ./results.csh | \grep MISS
MISS daley_intel_smoke_gx3_4x1_calcdragio complog ff88891 missing-data
MISS daley_intel_smoke_gx3_4x1_calcdragio compare ff88891  -1 -1 -1 missing-data

The missing data is for the test I'm adding in this PR. I've updated the PR description with this info.

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