-
Notifications
You must be signed in to change notification settings - Fork 139
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 computation of cdn_ocn for use in dynamics #771
Conversation
- Compute and used cdn_ocn on U, E, and N cell locations as needed for dynamics. - Add halo updates in dynamics before calling grid_average. This doesn't change answers, but is the safe thing to do in general. Performance does not seem to be affected.
Please check if this seems correct. This closes #737. |
I'm confused about when/how variable form drag is invoked -- this does not seem to be documented well in Icepack. E.g. |
The B cases do changes answers. Looking closer at output, I think we're getting a round off level change in the cdn_ocn when we average a constant field to another grid. On the first timestep, the only differences in the log file are in KE, ice spped, and arwt ocn heat flux and those are roundoff different in the global sum. The difference then grows. So I think that's consistent with our expectation and it's OK. I don't have any insight into the underlying computation of cdn_ocn, just focused on the grid location. Happy to make other changes, in documentation or otherwise, as needed. |
So on the B-grid it's a roundoff change... Is the "more substantial" tickmark above for C-grid simulations? I found the |
I called it "more substantial" because it will produce more-than-roundoff different results for non-constant cdn_ocn fields. Partly I didn't realize that cdn_ocn is a mostly constant. But I still think "more substantial" is the correct characterization, even if our tests happen to mostly (or always) use cdn_ocn=constant fields. We are using an "S" mapping to C grid too, so that should map a constant field to a constant field plus roundoff even for C grids. |
PR checklist
Update computation of cdn_ocn for use in dynamics
apcraig
All tests pass but answers have changed as expected, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#af8cc84308beb3dd959639f0142a8cbb29a9a5e6. QC tests pass,
How much do the PR code changes differ from the unmodified code?
Does this PR create or have dependencies on Icepack or any other models?
Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
Does this PR add any new test cases?
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.)
Please provide any additional information or relevant details below:
Compute and used cdn_ocn on U, E, and N cell locations as needed for dynamics.
Add halo updates in dynamics before calling grid_average. This doesn't change answers, but is the safe thing to do in general. Performance does not seem to be affected.