-
Notifications
You must be signed in to change notification settings - Fork 321
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
Rework cold start initialization of wa and zwt to try to make #523 bit-for-bit #577
Conversation
In the groundwater_irrigation branch (ESCOMP#523), @swensosc has stopped resetting wa_col each time step if use_aquifer_layer is false. However, this leads to having a substantially different value of wa_col when use_aquifer_layer is false: previously, it was reset to aquifer_water_baseline each time step, but with the changes from @swensosc, it stays close to its initial values, which have been 4000 in most places. This commit changes the initial values to match the value it was being reset to, so it simply starts at aquifer_water_baseline - so the every-time-step-resetting to aquifer_water_baseline can be removed without massively changing the value of wa_col. In addition to changing the cold start initialization of wa_col, we are also changing the cold start initialization of zwt in this case where use_aquifer_layer is false: The old initialization of zwt wouldn't work as intended now that we have changed the initial value of wa_col; @swensosc suggested this new initialization method instead. This initialization to zi(c,nbedrock(c)) is correct if there are no saturated layers, and close enough for a decent cold start even if there are.
@swensosc can you please take a look at the changes in this PR and let me know if I accurately implemented your suggestions? |
@swensosc the diffs I'm seeing in testing this branch (vs. master) are bigger than I expected. I'm not hugely concerned, but I would like to talk to you about this later today to get a second opinion; at the very least, I'd like you to carefully review my code changes to make sure I didn't mess something up (especially changes in SoilHydrologyInitTimeConst). Most diffs were in cold start tests, as expected. I spot-checked the diffs for one cold-start test: the three-day test,
However, there were also differences in transient cases that cross the year boundary. In retrospect, this is expected, because when a new column comes into existence, it will start with its new cold start values of wa and zwt. Six tests showed differences in this respect:
The biggest diffs in non-methane (and non-ERR*) fields were these, also from the SMS decStart test:
So not huge on the global average, but almost certainly bigger for select columns that newly came into existence (I haven't dug into this). Other than that short decStart test, the biggest non-methane, non-ERR diff from a non-cold-start test was (from
Again, my guess is that this is all reasonable, but I'd like to get your opinion on it... it's possible that we should do something like run the diagnostics package on a case before and after this change to ensure that we're getting the same climate, to make sure there isn't some unexpected sensitivity to these initial values. |
From @swensosc
and later:
|
This should have been added in the previous tag
Description of changes
Rework cold start initialization of wa and zwt to try to make #523 bit-for-bit. See commit message for c600499 for details.
Specific notes
Contributors other than yourself, if any: Changes guided by @swensosc
CTSM Issues Fixed (include github issue #): none
Are answers expected to change (and if so in what way)? YES: Answer changes expected for CLM50 cold start cases
Any User Interface Changes (namelist or namelist defaults changes)? No
Testing performed, if any:
aux_clm testing underway