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 Icepack to #5999551 including snowbrinebugs #797

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Nov 30, 2022

PR checklist

  • Short (1 sentence) summary of your PR:
    Update Icepack to #5999551 including snowbrinebugs
  • 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 suite run on cheyenne, all bit-for-bit except snwgrain cases, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#cb58257857d429c4c2dc4185d16c3f991e379271
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit except snwgrain (expected)
    • 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 advection description in document, closes #793

Update f_bound setting in ice_history.F90, closes #796

Update advection description in document

Update f_bound setting in ice_history.F90
@apcraig
Copy link
Contributor Author

apcraig commented Nov 30, 2022

Please review the changes to the documentation associated with #793. I tried to make an appropriate updates but I'm sure it's not quite right.

@apcraig
Copy link
Contributor Author

apcraig commented Dec 2, 2022

Looking for a review, would be nice to merge before the weekend. Thanks.

@@ -35,7 +35,7 @@ versions but have not yet been implemented.

Two transport schemes are available: upwind and the incremental
remapping scheme of :cite:`Dukowicz00` as modified for sea ice by
:cite:`Lipscomb04`. The upwind scheme is naturally suited for a C grid discretization. As such, the C grid velocity components (i.e. :math:`uvelE=u` at the E point and :math:`vvelN=v` at the N point) are directly passed to the upwind transport scheme. On the other hand, if the B grid is used, :math:`uvel` and :math:`vvel` (respectively :math:`u` and :math:`v` at the U point) are interpolated to the E and N points such that the upwind advection can be performed. Conversely, as the remapping scheme was originally developed for B grid applications, :math:`uvel` and :math:`vvel` are directly used for the advection. If the remapping scheme is used for the C grid, :math:`uvelE` and :math:`vvelN` are first interpolated to the U points before performing the advection.
:cite:`Lipscomb04`. The upwind scheme is naturally suited for a C grid discretization. As such, the C grid velocity components (i.e. :math:`uvelE=u` at the E point and :math:`vvelN=v` at the N point) are directly passed to the upwind transport scheme. On the other hand, if the B grid is used, :math:`uvelU` and :math:`vvelU` (respectively :math:`u` and :math:`v` at the U point) are interpolated to the E and N points such that the upwind advection can be performed. Conversely, as the remapping scheme was originally developed for B grid applications, :math:`uvelU` and :math:`vvelU` are directly used as departure points for the advection. If the remapping scheme is used for the C grid, :math:`uvelE` and :math:`vvelN` are first interpolated to the U points before performing the advection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JFLemieux73 @TillRasmussen: do you agree with this statement?

:cite:Lipscomb04. The upwind scheme is naturally suited for a C grid discretization. As such, the C grid velocity components (i.e. :math:uvelE=u at the E point and :math:vvelN=v at the N point) are directly passed to the upwind transport scheme. (Note however that the upwind scheme does not transport all potentially available tracers.) On the other hand, if the B grid is used, :math:uvelU and :math:vvelU (respectively :math:u and :math:v at the U point) are interpolated to the E and N points such that the upwind advection can be performed. Because the remapping scheme was originally developed for B grid applications, uvelU and vvelU are are interpolated to the E and N points internally in the advection code, when running on B grids. If the remapping scheme is used for the C grid, :math:uvelE and :math:vvelN are first interpolated to the U points before performing the advection when l_fixed_area (described below) is false.

Copy link
Contributor

@TillRasmussen TillRasmussen Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting from:
Because the remapping scheme was originally developed for B grid applications, uvelU and vvelU are are interpolated to the E and N points internally in the advection code, when running on B grids. If the remapping scheme is used for the C grid, :math:uvelE and :math:vvelN are first interpolated to the U points before performing the advection when l_fixed_area (described below) is false.

I would reformulate the above to
The remapping scheme was originally developed for B grid applications, which means that  :math:uvelU and :math:vvelU are needed. If l_fixed_area is true (by default hardcoded to false) then :math:uvelE and :math:vvelN are needed, which requires interpotation. If the remapping scheme is used for the C grid, :math:uvelE and :math:vvelN are first interpolated to the U points before performing the advection when l_fixed_area (described below) is false.

"Not to be included"
The interpolation on B grid from uvelU and vvelN are not needed in the current setting, however it is done anyway.

@apcraig
Copy link
Contributor Author

apcraig commented Dec 2, 2022

I significantly updated the advection scheme descriptions after trying to understand all the iterations. Hopefully it's clearer and correct. If not, we should continue to iterate.

@apcraig apcraig merged commit b16d7fd into CICE-Consortium:main Dec 2, 2022
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.

Found one remaining #ifdef ncdf not covered by USE_NETCDF Improve documentation for advection schemes
4 participants