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.

Sign up for GitHub

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

Jump to bottom

Update KE diagnostic #784

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Update KE diagnostic #784

merged 2 commits into from
Nov 7, 2022

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Nov 4, 2022 •
edited
Loading

PR checklist

  • Short (1 sentence) summary of your PR:
    Update KE diagnostic
  • 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.
    Full test suite passes, all results bit-for-bit except KE, rms ice speed, and max speed on C-grid in log files. https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#db254c11ba0a96b10f875a2d2003ec6aa9298073
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit science, a few diagnostics changed
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • 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:

Update KE diagnostic

  • Average U and V fields to T grid for KE calculation for B, C, and CD grid, raw U and V were combined with T grid variables in prior implementation.

Also

  • Update max speed calculation for C grid, calc speed at both E and N points
  • Refactor highfreq option u,v passed into icepack for windstress calculation
    • This is bit-for-bit
    • Create local variables uvelT_icep and vvelT_icep to store highfreq info
    • Compute [uvelT,vvelT]_icep at start of therm calc instead of end of dynamics calc
    • No longer need to recalc [uvelT,vvelT]_icep on restart
  • Update some indentation in ice_diagnostics.F90

Closes #734
See also #655

@apcraig
Update KE diagnostic
096320c 
- Average U and V fields to T grid for KE calculation for B, C, and CD grid,
  raw U and V were combined with T grid variables in prior implementation.

Also
- Update max speed calculation for C grid, calc speed at both E and N points
- Refactor highfreq option u,v passed into icepack for windstress calculation
  - This is bit-for-bit
  - Create local variables uvelT_icep and vvelT_icep to store highfreq info
  - Compute [uvelT,vvelT]_icep at start of therm calc instead of end of dynamics calc
  - No longer need to recalc [uvelT,vvelT]_icep on restart
- Update some indentation in ice_diagnostics.F90
@apcraig apcraig mentioned this pull request Nov 4, 2022
Closed
9 tasks
eclare108213
eclare108213 approved these changes Nov 4, 2022
View reviewed changes
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.

This might break restarts from previous-version restart files, but that will be fine going forward and otherwise it looks good. Thanks for cleaning this up.

@apcraig
Copy link
Contributor Author

apcraig commented Nov 5, 2022

This doesn't affect prior restarts. Nothing has changed on the restart file. With this update, we just compute two derived quantities in a different place, but it's still there.

@apcraig
Copy link
Contributor Author

apcraig commented Nov 5, 2022

Test results are here, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#db254c11ba0a96b10f875a2d2003ec6aa9298073.

@eclare108213
Copy link
Contributor

Looks good. Please merge when you're ready.

@apcraig apcraig merged commit aa1e066 into CICE-Consortium:main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eclare108213 eclare108213 eclare108213 approved these changes

@dabail10 dabail10 Awaiting requested review from dabail10

@JFLemieux73 JFLemieux73 Awaiting requested review from JFLemieux73

Assignees
No one assigned
Labels
Analysis
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kinetic energy diagnostic mixes T and U variables for B grid
2 participants
@apcraig @eclare108213