Added catch for unusual case of (hicen_init(n+1) - hicen_init(n))>0 #337
Conversation
|
@proteanplanet |
|
This is ready for review. Thanks for your patience. |
There was a problem hiding this comment.
Thank you @proteanplanet.
@dabail10, does this catch the original error (#333)? Is this also related to #338 or is that a different problem?
columnphysics/icepack_therm_itd.F90
Outdated
| ! interpolate between adjacent category growth rates | ||
| slope = (dhicen(n+1) - dhicen(n)) / & | ||
|
|
||
| if (hicen_init(n+1) - hicen_init(n))>0 |
There was a problem hiding this comment.
This should be c0 I believe.
There was a problem hiding this comment.
Did you try compiling this? I believe it should be:
if ((hicen_init(n+1) - hicen_init(n))>c0) then
There was a problem hiding this comment.
The fortran issues were fixed earlier - looks like you needed to refresh, @dabail10 ?
columnphysics/icepack_therm_itd.F90
Outdated
|
|
||
| else | ||
|
|
||
| write(warnstr,*) subname,... |
There was a problem hiding this comment.
So, we want the code to abort in this case?
There was a problem hiding this comment.
Should this not be an ampersand for continuation?
|
Has anyone run with this change? Is it bit-for-bit? |
|
It should be BFB but I don't think it's been tested other than the automated check above. Would be good to do a full suite on it, for sanity's sake. Maybe pull in the other presumed-BFB Icepack change and test both at the same time? |
Yes. I have. It is BFB for a one-year test of Icepack on Cori. |
|
The icepack test suite is bit-for-bit with this PR. Will test this in CICE next. |
|
The CICE test suite is bit-for-bit with this PR, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#714dba689bfd2ab53298c93206add1e90bf1de3a. |
Summary: This draft pull request addresses, in part, issue #333, where an erroneous initial condition of (hicen_init(n+1) - hicen_init(n))=0 made its way to the thermodynamic redistribution code.
Developers: @proteanplanet, @dabail10, @whlipscomb, @eclare108213
Testing: Testing has not been completed until feedback is obtained on whether we desire this solution to part of the referred issue.
Changes: The code should be BFB with this change.
Dependencies: None added.
Documentation: None needs to be added with this change.
Additional information: Note that the issue also highlighted the problem of a CFL-like violation in ice thickness redistribution, but this PR does not address that larger science issue. Instead, the CFL-like violation is currently catered for by switching to rebinning redistribution (remap_flag = .false.) when linear redistribution occasionally breaks down. Fixing this is a different problem.