-
Notifications
You must be signed in to change notification settings - Fork 0
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
Subroutines for CD-grid rheology #10
Conversation
supersedes #9 |
subroutine deformations_T (nx_block, ny_block, & | ||
icellt, & | ||
indxti, indxtj, & | ||
uvelE, vvelE, & | ||
uvelN, vvelN, & | ||
dxN, dyE, & | ||
dxT, dyT, & | ||
tarear, & | ||
shear, divu, & | ||
rdg_conv, rdg_shear ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole subroutine looks like it's the same as deformations
, only calling strain_ratesT
instead of strain_rates
.
Should we instead keep the existing deformations
subroutine, and add an if (grid_system == 'B') .. else ..
block for calling strain_rates
vs strain_ratesT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eyes, @phil-blain .
I would prefer to reuse as much code as possible. Initially it's fine to have duplicative code, while developing, but if it makes sense and is easy to merge/clean up enroute, let's do it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, upon further inspection with @JFLemieux73, it's not exactly the same, as the B grid version does the average of the 4 corners:
https://github.com/CICE-Consortium/CICE/blob/0f75f068d63725062da4ea944443fbdd6dbed2fb/cicecore/cicedynB/dynamics/ice_dyn_shared.F90#L1269-L1278
whereas the CD-grid version already has the quantities at the tracer point, so it just computes the deformations without averaging:
https://github.com/CICE-Consortium/CICE/blob/0f75f068d63725062da4ea944443fbdd6dbed2fb/cicecore/cicedynB/dynamics/ice_dyn_shared.F90#L1363-L1370
Sorry, merged before I realized there was some discussion. I can unmerge, but propose we consider @phil-blain suggestion and create another PR if we want to make some changes. |
@JFLemieux73 what do you think? |
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
ENTER INFORMATION HERE
ENTER INFORMATION HERE
ENTER INFORMATION HERE