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

Merge Icepack Consortium #cabfe0f111b61057db16c Nov 20, 2022 #16

Conversation

apcraig
Copy link
Collaborator

@apcraig apcraig commented Nov 18, 2022

PR checklist

  • Short (1 sentence) summary of your PR:
    Merge Icepack Consortium #cabfe0f111b61057db16c Nov 20, 2022

  • Developer(s):
    apcraig

  • 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.
    Underway

  • 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 CICE 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/.)

    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

  • Minor update to fsd and fsd documentation

  • Fully deprecate 0-layer thermo and cesm ponds

  • Fix snow grain radius bug

  • Initial deprecation of zsalinty

  • Documentation

  • Modify fsd12 test run length (add short)

lettie-roach and others added 8 commits October 12, 2022 11:29
* Clarifications to documentation

* Edit documentation

* Update docs

* Add space
…tium#409)

* Clarifications to documentation

* Edit documentation

* Update docs

* Add space

* Add warning for waves=F and FSD=T

* Clarify documentation on wave forcing
…onsortium#411)

* finalize 0-layer thermo and cesm ponds deprecation

* set default hs0=0 and document

* formatting table in doc

* revert default hs0 = 0.03
* Initial, partial deprecation of zsalinity

* remove zsal tests

* make icepack_warnings_getall public
@apcraig apcraig requested a review from eclare108213 November 18, 2022 19:14
@apcraig
Copy link
Collaborator Author

apcraig commented Nov 18, 2022

I can reproduce results on Consortium Icepack main bit-for-bit EXCEPT the modal aero case (modal_aero=.true.). I'm trying to figure out if that's expected. Was anything put into this branch that changes modal_aero results that may be missing in the Consortium main? Of coarse, most of the implementation is in icepack_shortwave which is more difficult to compare. Any ideas?

@apcraig
Copy link
Collaborator Author

apcraig commented Nov 18, 2022

It looks like it's not modal_aero that is different. If I turn that off in both the main and E3SM modal test, answers are still different. In fact, in main, the answer is unchanged with modal_aero on or off.

If I then turn off tr_aero, main and E3SM branches now produce identical answers. So I guess the difference is more generally in tr_aero rather than modal_aero. Was something fixed in the E3SM branch that was no fixed in the main branch for tr_aero/modal_aero?

I'm inclined to merge this, but will wait for other ideas. My sense is that the modal_aero on main is broken (setting it has no impact). That's not true on the E3SM branch, so I assume it may be working better (correctly?) there?

@eclare108213
Copy link
Collaborator

We will fix modal_aero when we do the rest of the BGC. I'm not surprised that it's not working here - apparently it's only been run with tr_zaero and 5-band dEdd in E3SM.

The only testing option with tr_aero=T is set_nml.modal, which came in with CICE-Consortium#400. That's recent and we don't have a lot of experience with it since then. I don't see what could be causing the answer changes. We did have answer changes for tr_aero=T in CICE-Consortium/CICE#760, which presumably came from Icepack although there are other changes there that might have contributed (e.g. ice _forcing_bgc).

Could it be related to tfrz_option in set_nml.modal?

Here are the diffs I'm looking at -- are these the right ones?

I'm also inclined to merge this and keep going, but it's a little bothersome. At what point were the answers all BFB?

@apcraig
Copy link
Collaborator Author

apcraig commented Nov 19, 2022

I've set the trfrz_option to "_old" as needed. It's not that.

My brain is about to explode again, too many version of Icepack, CICE, and CICE with Icepack on Consortium and E3SM branch. It's extremely challenging to do this syncing and testing. I'm merging the latest Icepack into CICE in the Consortium now for weekend testing. So then we'll at least have the latest version of Icepack in CICE on the Consortium. And we'll have E3SM branch with the latest version of Icepack merged. I'll need to create a version of E3SM CICE syncing off Consortium and updating to the latest E3SM Icepack to test that too. And I am trying to test against the prior version of each branch and against the version on the other branch. I'll have more info soon.

@apcraig
Copy link
Collaborator Author

apcraig commented Nov 20, 2022

The divergence happens after 15 days and only in the full_ITD case. The diff on the first timestep is below. Looks bigger than roundoff, but it's bit-for-bit for 15 days. Is any modal forcing data monthly? Also tested in CICE and all results are bit-for-bit, but tests are usually for 10 days or less.

Out of all Icepack and CICE tests, this is the only test that is NOT bit-for-bit between Consortium main and E3SM branches (after PRs still-to-be-merged). I think maybe we should resync everything between Consortium main and E3SM branches again now. Everything is bit-for-bit except the Icepack modal tests, and we can review that diff once we have some clean versions.

< area fraction          =        0.99368350678762130
< avg ice thickness (m)  =        2.66925643402191337
< avg snow depth (m)     =        0.23099994625827652
< avg salinity (ppt)     =        2.78648573097112751
< avg brine thickness (m)=        0.00000000000000000
< surface temperature(C) =      -31.88261108611343175
< absorbed shortwave flx =        0.00043697248269911
< outward longwave flx   =     -190.53946259772979488
< sensible heat flx      =        2.19506253105318283
< latent heat flx        =       -1.10610940162935711
< subl/cond (m ice)      =       -0.00000150592622355
---
> area fraction          =        0.99368350678762118
> avg ice thickness (m)  =        2.66925643402166601
> avg snow depth (m)     =        0.23099994625934808
> avg salinity (ppt)     =        2.78648573096818408
> avg brine thickness (m)=        0.00000000000000000
> surface temperature(C) =      -31.88261169785598170
> absorbed shortwave flx =        0.00042924765373049
> outward longwave flx   =     -190.53946070024056780
> sensible heat flx      =        2.19506775929270503
> latent heat flx        =       -1.10610911993030525
> subl/cond (m ice)      =       -0.00000150592584037
12836c12836
< congelation (m)        =        0.00022353041869676
---
> congelation (m)        =        0.00022353041845022
12838,12841c12838,12841
< snow change (m)        =        0.00000715533851320
< effective dhi (m)      =        0.00022439677623476
< effective dhs (m)      =        0.00000607354218035
< intnl enrgy chng(W/m^2)=       24.31012928297122144
---
> snow change (m)        =        0.00000715533957798
> effective dhi (m)      =        0.00022439677598873
> effective dhs (m)      =        0.00000607354324514
> intnl enrgy chng(W/m^2)=       24.31012965838114326
12846c12846
< heat used (W/m^2)      =        2.40513870230475302
---
> heat used (W/m^2)      =        2.40513869973424343

@apcraig apcraig changed the title Merge Icepack Consortium #493373809c85561f Nov 18, 2022 Merge Icepack Consortium #cabfe0f111b61057db16c Nov 20, 2022 Nov 20, 2022
@apcraig
Copy link
Collaborator Author

apcraig commented Nov 20, 2022

Just pull minor update from Consortium main, #cabfe0f111b61057db16c, to change fsd12 to longer walltime for testing. Was timing out intermittently. Will retest then merge.

@apcraig apcraig marked this pull request as ready for review November 20, 2022 18:21
@apcraig apcraig merged commit 4d131da into E3SM-Project:cice-consortium/E3SM-icepack-initial-integration Nov 21, 2022
@apcraig
Copy link
Collaborator Author

apcraig commented Nov 21, 2022

E3SM Icepack (https://github.com/E3SM-Project/Icepack/tree/cice-consortium/E3SM-icepack-initial-integration) #4d131da, Nov 20, 2022 is bit-for-bit with CICE-Consortium Icepack (https://github.com/CICE-Consortium/Icepack) #cabfe0f, Nov 20, 2022 except for the modal aerosol tests which diverges in the full_ITD case after 15 days. I'm not sure we had done standalone Icepack modal aerosol tests between the two versions prior to now.

E3SM CICE (https://github.com/apcraig/CICE/tree/E3SMIcepackDEV) #ec8632f, Nov 21, 2022 is bit-for-bit with CICE-Consortium CICE (https://github.com/CICE-Consortium/CICE) #cb58527, Nov 20, 2022. However, the restart files may be different in Tsfc in ice-free ocean areas. This does not affect science but does affect ability to do regression testing using the restart files. Log files are also compared in that case and they indicate results are scientifically identical. These versions of CICE include the respective Icepack versions above. One thing to note, the modal aerosol tests in CICE are typically only 5 or 10 days, I don't know if that is hiding the bit-for-bit difference we see above after 15 days.

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.

4 participants