-
Notifications
You must be signed in to change notification settings - Fork 134
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
Corrects thin ice/snow treatment of enthalpy and other tracers #454
Conversation
Adopted from E3SM-Project/E3SM#5630 "This fix redistributes enthalpy and other tracers evenly in snow and ice when their respective thicknesses are < 1e-15 . Previously, these tracers were zero'd non-conservatively. Also corrects a bug in the zeroing of snow thicknesses, and removes snow in thickness_changes if the ice vanishes."
@eclare108213 @njeffery, please have a look at this implementation. I will complete testing and clean up the code once you agree it seems right. In particular, have a look at icepack_therm_vertical, lines 1714:1728. I show the old code from the E3SM columnphysics there as well. In the old code, there was a "tr_snow" and "tr_rsnw". We don't use that anymore. Is "snowgrain" the appropriate if-test for four lines of code in that section? Or is some other logic needed? |
do k = 1, nslyr | ||
fhocnn = fhocnn & | ||
+ zqsn(k)*hsn/(real(nslyr,kind=dbl_kind)*dt) | ||
zqsn(k) = -rhos*Lfresh | ||
!tcx, tcraig, in columnphysics, this is | ||
! is it correct that now everything is "if snwgrain"? |
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.
Yes, I think snwgrain
is the correct wrapper for these variables.
Test results look fine. Everything is running, Icepack is bit-for-bit, CICE is not. |
Is it worth running the QC tests on this, since it's not BFB? I think it's fine without them since the cases when the changes occur ought to be rare, but it would provide some extra confidence. |
Good idea about QC. QC passes with very small differences (this is a standard test with snwgrain off)
|
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.
Perfect, thanks for running the QC test.
PR checklist
Corrects thin ice/snow treatment of enthalpy and other tracers
njeffery
Icepack is bit-for-bit, CICE is not. https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#7e0821dcfe824ae99a3bf0cb957497a8e10c4da7
https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#e8a69abde90b99fc6528d469b8698506a99f6e2a
Adopted from E3SM-Project/E3SM#5630
"This fix redistributes enthalpy and other tracers evenly in snow and ice when their respective thicknesses are < 1e-15 . Previously, these tracers were zero'd non-conservatively. Also corrects a bug in the zeroing of snow thicknesses, and removes snow in thickness_changes if the ice vanishes."