Make dsnow and dsnown optional #506
Conversation
|
Before we merge here, maybe we should make sure they work properly with the Icepack driver and/or CICE with whatever changes might be implemented there. |
|
I did run the base_suite here. Thanks for the heads up on the l_dsnow and l_dsnown. |
|
Running a base_suite in CICE to make sure. |
|
Here are the CICE test results. Everything is bfb. |
| @@ -2589,7 +2601,8 @@ subroutine icepack_step_therm1(dt, & | |||
| meltbn (n) = c0 | |||
| congeln(n) = c0 | |||
| snoicen(n) = c0 | |||
| dsnown (n) = c0 | |||
| l_dsnown = c0 | |||
| if (present(dsnown)) l_dsnown = dsnown (n) | |||
There was a problem hiding this comment.
If the old implementation, "dsnown(n)" was always set to zero before being passed into thermo_vertical. Now it is being initialized to the incoming value of "dsnown(n)". Is this correct? Why doesn't this change answers?
There was a problem hiding this comment.
This confused me as well, but dsnow and dsnown are both intent(inout). I guess the initialization to zero is automatically happening in the driver and we were never using it in the driver. I will fix this though.
There was a problem hiding this comment.
I am thinking that dsnown should be intent out only.
There was a problem hiding this comment.
Also, dsnown (called dsnow) is initialized to zero in thermo_vertical. Again, it should really only be intent(out).
There was a problem hiding this comment.
@dabail10, Please make updates ASAP if possible, would like to get this merged and into weekend testing as first steps to release. Thanks.
|
I have just pushed the change. |
| @@ -2589,7 +2601,8 @@ subroutine icepack_step_therm1(dt, & | |||
| meltbn (n) = c0 | |||
| congeln(n) = c0 | |||
| snoicen(n) = c0 | |||
| dsnown (n) = c0 | |||
| l_dsnown = c0 | |||
| if (present(dsnown)) dsnown(n) = c0 | |||
There was a problem hiding this comment.
This line is redundant, you should remove it.
|
I'm going to merge this then remove the redundant line when I update the version number. |
Update interface documentation Clean up redundant dsnown line in icepack_therm_vertical, see CICE-Consortium#506.
Update interface documentation Clean up redundant dsnown line in icepack_therm_vertical, see #506.
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
Reopened this one. This solves issue Make dsnow and dsnown optional. #492
dabail10 (D. Bailey)
https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash#286630fdecc8efc4dcf4bd7ecf2315101c69c391
This is the icepack part of making both dsnow and dsnown optional. Later I can add some diagnostics for these in CICE.