Thermo itd2 #25
Thermo itd2 #25
Conversation
eclare108213
requested review from
darincomeau,
njeffery,
proteanplanet and
erinethomas
|
the wave sections look good to me :) |
|
CICE-QC testing passed. |
There was a problem hiding this comment.
I tested this branch with column_package and icepack options in the 1-year stand-alone mpas-seaice test with snicar-ad active. Although the results now have nonBFB differences from snicar, the latest changes are bfb when contrasting icepack_step_therm2 with colpkg_step_therm2. I also tested the column changes suggested for ice_itd and ice_therm_itd. Both of these are bfb in the stand alone 1-year test.
|
@njeffery thank you for the additional testing! Just to clarify
Was this with the code as it is now, or with the proposed changes in column/ turned on (uncommented)? They are commented out at the moment because I wanted to see how big a difference there might be in the 5-year CICE-QC tests, comparing the new icepack calls with column prior to that change. The next question is: Should I uncomment the proposed changes in column/ or leave them like this, just as a reference? |
|
@eclare108213 : This is uncommenting the proposed changes and seeing if they make a difference for the column_package run. They didn't. That's not to say they won't in a 5-year run. If they make a difference in the D-tests and the CICE-QC then I think we should add them in the integration branch (with documentation comments). This way we'll have a clean test for assessing the impacts of upcoming changes. |
|
They were non-BFB in D-cases. I didn't test them in CICE-QC, since I wanted to see max differences, and presumably these changes to column will make the codes more similar. So your suggestion is to uncomment and merge them into the icepack-integration branch, and not do a separate PR into E3SM with them? |
|
I'm still on the fence. I'm not sure that a PR into E3SM at this point is worth it if the impacts are not climate changing. It'll slow down the integration, and we could always do one later. |
|
I have modified column/ice_itd.F90 code to include the icepack changes. I left the change in column_ice_therm_itd.F90 commented out, since Tliquidus_max is not defined in column and its default value in icepack is 0 (which explains why tests with and without it using icepack are BFB). I think this is ready to merge - if anyone disagrees, speak up soon! |
This is to incorporate v3atm's PR #25. The in-cloud properties should be outputted after the cloud microphysics tendencies are taken into account, whereas they are outputted in P3 without accounting for the microphysics calculations. This fix does not affect model simulations, but will affect all of offline analysis where in-cloud property is used. [non-BFB]
Call icepack_step_therm2.
This PR is non-BFB for Icepack, due to the changes shown in column/ice_itd.F90. This bug fix keeps the time level of the state variables consistent during ITD remapping in thickness space. See CICE-Consortium/CICE#87 and CICE-Consortium/Icepack#222 for further information. QC testing in CICE indicated that the bug fix was not climate-changing.
One line was added as a comment in column/ice_therm_itd.F90, from Icepack. This change was BFB in 3-month (icepack) D cases but might not be BFB in longer simulations. A maximum liquidus temperature is not defined in colpkg.
Three constants from ice_colpkg_shared will need to be moved elsewhere, eventually. Their (new) CamelCase equivalents are commented out for now, pending a decision on how to handle such constants.
nBioLayers and nAerosols are currently required by Icepack - the BGC interface will be cleaned up when we start merging the BGC code.
This PR includes some FSD and wave code in shared/mpas_seaice_icepack.F90, all of which is commented out for now. Thanks to @erinethomas for the head start on this module!
CICE-QC testing on the current code is underway.