-
Notifications
You must be signed in to change notification settings - Fork 138
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
CMIP changes #219
CMIP changes #219
Conversation
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.
I think we need a different variable name for Tsnic. I interpreted it differently: Ts surface temp, n on categories, i ice... what does the c stand for? Celcius? I guess sn is supposed to be snow. Maybe Tis_intfc? What does the CMIP6 standard want it to be called?
Otherwise these changes all look fine to me. Does the documentation need to be updated?
DMI would like to merge their (CICE) code with dynamic allocation of variables in the next few weeks, and they'll start with CICE master. If there are a bunch of new history variables to add, we should try to do that first to avoid conflicts. |
I was thinking T sn(ow) ic(e). Would you like Tsnowice better? I would definitely like the CMIP-CICE changes to go in soon. I need the first pull request with the dummy stuff merged before I can do this though. The documentation will only need to be updated for CICE I believe, not icepack. |
Tsnowice sounds like we have a temperature for a snow-ice layer, which we don't resolve (yet!). How about Tinterface? I'd turn that into Tintfc... |
I was concerned about the use of intfc, because if the icepack_intfc.F90 module. |
Ah. Too few words to describe different things. How about Tsi_interface, then? Or Tsnice if that's not already in use (it looks kind of familiar). |
I did not find Tsnice anywhere, so that is good. Dave |
Renamed Tsnic to Tsnice. |
@eclare108213, can you approve or comment then I'll start merging. |
This looks fine to me. There is a single change to the shortwave module, which is commented out. It's better not to have stuff like that in there unless there's a meaningful reason for it. But I'm okay with merging this now. |
This changes answers, so I am waiting to commit it separately. Dave |
Just to be clear, it's that shortwave change that is commented out that changes answers, right? That's a little scary, it's just an intent change. Does is change science? I guess we can talk about it when the change is proposed later. |
Exactly. I am unsure so far why it changes answers. It is something to do with the setting the initial snow depth and fraction related to the level ponds. These are set to zero before the call to shortwave_dEdd_set_snow, so they should be intent(inout) in this subroutine. |
I just had a quick look at the code too. I agree they should be in/out. But it's odd that it changes answers and that the intent issue wasn't picked up before now. Interesting. thanks. |
The intent inout was only picked up by uninitialized variable checking with the NAG compiler. |
This PR is necessary for the CMIP changes (CICE Issue 93) and also add changes for the evaporation over snow and sea ice issue (CICE issue 97). The CICE related CMIP changes are coming in a future PR. Here is a short summary of what has changed:
fcondbot and fcondbotn are added so they can be shared with the CICE code.
Tbot is now shared with the driver and hence CICE.
A new diagnostic variable Tsnic has been added for the snow-ice interface temperature.
New variables evaps and evapi along with category versions have been added to keep track of evaporation over snow and bare ice separately.
Developer(s): D. Bailey
Please suggest code Pull Request reviewers in the column at right.
Are the code changes bit for bit, different at roundoff level, or more substantial? BFB
Is the documentation being updated with this PR? (Y/N) N
If not, does the documentation need to be updated separately at a later time? (Y/N) N
Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-icepack/.
Other Relevant Details: