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

finalize 0-layer thermo and cesm ponds deprecation in Icepack #411

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

eclare108213
Copy link
Contributor

@eclare108213 eclare108213 commented Nov 11, 2022

  • Short (1 sentence) summary of your PR:
    Removes all ifdef options allowing 'undeprecation' of 0-layer thermo and cesm meltponds, and updates documentation
  • Developer(s):
    @eclare108213 @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.
    Full test suite on cheyenne, intel is bfb. https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#ef0efc78666d7b7451eb86a76e2eb0e35f50156a
  • 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:

@dabail10
Copy link
Contributor

I think that hs0 is only used for the CESM ponds. hs0 is used to initialize the snow fraction in shortwave_dEdd_set_snow which is still called in the level ponds. However, with hs0 set to 0, the snow fraction will be set to 1 and then it is recomputed using hs1. Not sure if this subroutine should even be called.

@eclare108213
Copy link
Contributor Author

@dabail10 I'll take a look, thanks for the insight.

@eclare108213
Copy link
Contributor Author

I need to fix the formatting in the documentation, both here and in CICE-Consortium/CICE#787

@eclare108213
Copy link
Contributor Author

Unfortunately LANL is blocking my access to the Icepack documentation generated from this PR. Could someone take a look at section 2.7 (there should be a new subsection called 'Snow fraction') and the bottom of the variable index, and let me know if they formatted okay?

@apcraig
Copy link
Contributor

apcraig commented Nov 11, 2022

Everything looks good to me. I tried to grab the pdf to send to you, but I think pdfs are only generated with a tag or something.

@eclare108213
Copy link
Contributor Author

I reverted the default/initial values of hs0 in both Icepack here and CICE CICE-Consortium/CICE#787, but otherwise did not alter the documentation updates in either PR, pending resolution of CICE-Consortium/CICE#635. This PR partially addresses the confusing documentation called out in that PR.

@apcraig apcraig merged commit 18fc1c9 into CICE-Consortium:main Nov 15, 2022
@apcraig
Copy link
Contributor

apcraig commented Nov 15, 2022

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