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 documentation #32

Conversation

apcraig
Copy link
Collaborator

@apcraig apcraig commented Sep 27, 2023

PR checklist

  • Short (1 sentence) summary of your PR:
    Update documentation prior to merging to Consortium
  • 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.
    no testing done, changes only to documentation
  • 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:

This primarily updates the Icepack public interfaces. I checked the rest of the documentation, all new namelist and CPPs are already documented. The shortwave changes on this branch are also documented. Are there other things that are missing in the documentation? Ultimately, this is trying to fix any missing documentation from this PR, CICE-Consortium#460

@apcraig
Copy link
Collaborator Author

apcraig commented Sep 27, 2023

I've activated this branch in readthedocs. You can see the latest documentation associated with this PR here, https://apcraig-icepack.readthedocs.io/en/e3smdocs1/

Copy link
Collaborator

@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.

Things to check/fix if needed :

  • make sure Tf options are explained in the text
  • hi_min - note in the PR that default might change answers in non-CESM configurations, but now it's a namelist option. Add note about it in main text.
  • Tliquidus_max - note in PR the original value and that it's now in namelist. Add note about it in main text.

@apcraig
Copy link
Collaborator Author

apcraig commented Sep 28, 2023

I updated the PR about hi_min and Tliquidus_max. I also took a shot at updating the documentation with additional info about Tf, hi_min, and Tliquidus_max. But those might be wrong, please have a look and suggest other wording and especially if the changes belong in a different part of the user guide.

Copy link
Collaborator

@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.

Nice detective work! This looks good to me. Thank you.

@eclare108213 eclare108213 merged commit e33cc75 into E3SM-Project:cice-consortium/E3SM-icepack-initial-integration Sep 29, 2023
1 check passed
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.

2 participants