Skip to content

Fixes for 'umaskCD and more'#39

Closed
eclare108213 wants to merge 8 commits into
apcraig:cgridDEVfrom
eclare108213:umaskCD
Closed

Fixes for 'umaskCD and more'#39
eclare108213 wants to merge 8 commits into
apcraig:cgridDEVfrom
eclare108213:umaskCD

Conversation

@eclare108213
Copy link
Copy Markdown
Collaborator

@eclare108213 eclare108213 commented Nov 20, 2021

  • Short (1 sentence) summary of your PR:
    Draft fixes for @TillRasmussen 's PR
  • Developer(s):
    @TillRasmussen
  • 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.
    ENTER INFORMATION HERE
  • 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 Icepack 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/. 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:

TillRasmussen and others added 8 commits November 19, 2021 20:38
@TillRasmussen @eclare108213
Changes in ice_dyn* are interpolation of uvelE/vvelN to B grid. ice_t…
83e2bd1 
…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.
@TillRasmussen @eclare108213
changed according to comments. changed average from F to S and. comme…
85e7782 
…nted out in vp and eap
@TillRasmussen @eclare108213
Changes in ice_dyn* are interpolation of uvelE/vvelN to B grid. ice_t…
06220ae 
…ransport files are changed in a way so that velocities are interpolated to b grid for depature point function and kept at E or N grid possible.
@TillRasmussen @eclare108213
changed according to comments. changed average from F to S and. comme…
f07cbc3 
…nted out in vp and eap
@TillRasmussen @eclare108213
Create local uvmCD that is projected into umaskCD and icellu.
14fda22 
Added function to fint zeta, eta and replacement pressure at U
@TillRasmussen @eclare108213
Added call to routine
bbd05b9
@TillRasmussen @eclare108213
bux fixes
9a8edcc
@eclare108213
debugging array issues
1d89af7
eclare108213
eclare108213 commented Nov 20, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

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

just adding a few comments on things I noticed

Comment thread cicecore/cicedynB/dynamics/ice_dyn_evp.F90
nmassdti (:,:,iblk), fcorN_blk (:,:,iblk), &
nmask (:,:,iblk), &
nmassdti (:,:,iblk), fcorN_blk (:,:,iblk),&
nmask (:,:,iblk), nmask (:,:,iblk), &
Copy link
Copy Markdown
Collaborator Author

@eclare108213 eclare108213 Nov 20, 2021

Choose a reason for hiding this comment

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

maybe the second mask in this line could be made into an optional variable

Copy link
Copy Markdown

@TillRasmussen TillRasmussen Nov 20, 2021

Choose a reason for hiding this comment

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

I think that I can eliminate the second mask and differentiate the two mask in the call as it is done for dyn_prep2 on E and N grid

Comment thread cicecore/cicedynB/dynamics/ice_dyn_shared.F90
tmpcalcse = strength/(Deltase + tinyarea)
endif
tmpcalcne = capping*(strength/max(Deltane, tinyarea))+ &
(1-capping)*strength/(Deltane + tinyarea)
Copy link
Copy Markdown
Collaborator Author

@eclare108213 eclare108213 Nov 20, 2021

Choose a reason for hiding this comment

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

need to use c1 instead of 1

Comment thread cicecore/cicedynB/dynamics/ice_dyn_shared.F90


real (kind=dbl_kind), intent(in):: &
zetax2T_00,zetax2T_10,zetax2T_11,zetax2T_01, &
Copy link
Copy Markdown
Collaborator Author

@eclare108213 eclare108213 Nov 20, 2021

Choose a reason for hiding this comment

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

At some point, add comments at the end of each line to identify the variables (with units).

eclare108213
eclare108213 commented Nov 20, 2021
View reviewed changes
Comment thread cicecore/cicedynB/dynamics/ice_dyn_evp.F90
call viscous_coeffs_and_rep_pressure_T2U (zetax2T(i ,j ), zetax2T(i ,j+1), &
zetax2T(i+1,j+1), zetax2T(i+1,j ), &
etax2T (i ,j ), etax2T (i ,j+1), &
etax2T (i+1,j+1), etax2T (i+1,j ), &
Copy link
Copy Markdown
Collaborator Author

@eclare108213 eclare108213 Nov 20, 2021

Choose a reason for hiding this comment

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

This is an example how I prefer to align code by each element (parentheses, i's, j's, commas, &'s). The last line of this argument list still needs to be aligned.

@eclare108213 eclare108213 mentioned this pull request Nov 20, 2021
Merged
16 tasks
@apcraig apcraig closed this Nov 22, 2021
@apcraig
Copy link
Copy Markdown
Owner

apcraig commented Nov 22, 2021

Closed. This PR was merged into #33.

@eclare108213 eclare108213 deleted the umaskCD branch December 4, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@TillRasmussen TillRasmussen TillRasmussen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eclare108213 @apcraig @TillRasmussen