-
Notifications
You must be signed in to change notification settings - Fork 51
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
Fix and improve anomaly forcings for ISSP cases #292
base: main
Are you sure you want to change the base?
Conversation
@ekluzek, a few questions:
<default_value>''</default_value>
<value compset="^SSP126_">Anomaly.Forcing.cmip6.ssp126</value> |
@samsrabin is this ready- if not can you make it a draft please. |
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.
It's not clear to me that anomaly_forcing will always be defined and be an array.
Force-pushed to remove premature merge of main into this branch, which was interfering with my testing. |
@ekluzek This is ready, if you'd like to review! |
Note that this is up-to-date with cdeps1.0.34. If you'd like me to merge in the latest tag, let me know and I'll redo the testing. |
@jedwards4b This is ready to review, and @ekluzek will also make some time to look at it in the next few days. It would be nice if we can get it in soon. |
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.
Really glad you took this on. This is so great to have coming in. I'm suggesting some changes.
I think it would be better to be data driven rather than code driven. And I layout a mostly fleshed out plan on how to do that. You'll need to make sure that all works, but I think it will and it's an improvement over some of the limitations of a part of what's in here.
We should get together to go over all of this. I also have some questions about one set of changes that would be good for you to explain.
if not anomaly_forcing or anomaly_forcing[0] is None: | ||
# If not in namelist, check whether it's an SSP compset | ||
ssp = re.search(r"^SSP\d+_DATM", compset) | ||
if ssp: |
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.
As I understand this -- it will ensure anomaly_forcing is forced to be on for SSP compsets. But, I don't think that's what we want. I think that the default certainly should be for it to be on, but the user should be able to turn it off if they want. The AF forcing is also specific to the DATM_MODE so it should only do this depending on both the compset and DATM_MODE. And since the DATM_MODE the AF forcing matches will be changing, it would be better for this to be in the XML data rather than in the python code. Better to be data driven when possible.
As such I'm thinking this part should be reverted.
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.
See below for what I suggest to add to the namelist_definition file. We will also need to make sure compset is a valid field in the namelist defintion file.
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.
The anomaly_forcing
option none
is designed to handle situations where the user wants to turn off anomaly forcing for an SSP compset. See diff below:
anomaly_forcing = nmlgen.get_value("anomaly_forcing")
- if anomaly_forcing[0] is not None:
+ if anomaly_forcing[0] is not None and anomaly_forcing[0] != "none":
streamlist += anomaly_forcing
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.
This is confirmed by comparing the namelists from the CTSM branch's tests without and with the new clm-noAnomalyForcing
testdef:
suitedir="/glade/derecho/scratch/samrabin/tests_0219-152958de"
cd "${suitedir}"
testdir_anom_on="SMS_D_Ld5.f10_f10_mg37.ISSP245Clm50BgcCrop.derecho_intel.clm-default.0219-152958de_int"
testdir_anom_off="${testdir_anom_on/clm-default/clm-default--clm-noAnomalyForcing}"
cd "${testdir_anom_on}/CaseDocs"
for f in *; do
echo $f
diff $f ../../${testdir_anom_off}/CaseDocs/$f
echo " "; echo " "
done
Relevant outputs:
datm_in
12c12
< anomaly_forcing = "Anomaly.Forcing.cmip6.ssp245"
---
> anomaly_forcing = 'none'
datm.streams.xml
698,726d697
<
< <stream_info name="Anomaly.Forcing.cmip6.ssp245">
< <taxmode>cycle</taxmode>
< <tintalgo>nearest</tintalgo>
< <readmode>single</readmode>
< <mapalgo>bilinear</mapalgo>
< <dtlimit>1.5</dtlimit>
< <year_first>2015</year_first>
< <year_last>2100</year_last>
< <year_align>2015</year_align>
< <vectors>null</vectors>
< <meshfile>/glade/campaign/cesm/cesmdata/inputdata/share/meshes/fv0.9x1.25_141008_polemod_ESMFmesh.nc</meshfile>
< <lev_dimname>null</lev_dimname>
< <datafiles>
< <file>/glade/campaign/cesm/cesmdata/inputdata/atm/datm7/anomaly_forcing/CMIP6-SSP2-4.5/af.allvars.CESM.SSP2-4.5.2015-2100_c20220628.nc</file>
< </datafiles>
< <datavars>
< <var>huss Sa_shum_af</var>
< <var>pr Faxa_prec_af</var>
< <var>ps Sa_pbot_af</var>
< <var>rlds Faxa_lwdn_af</var>
< <var>rsds Faxa_swdn_af</var>
< <var>tas Sa_tbot_af</var>
< <var>uas Sa_u_af</var>
< <var>vas Sa_v_af</var>
< </datavars>
< <offset>0</offset>
< </stream_info>
<
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.
@ekluzek Pinging, as discussed.
@@ -224,7 +224,7 @@ | |||
<type>char(10)</type> |
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.
Near the top of the file streamslist is being set with a setting for GSWP3 like this:
<value datm_mode="CLMGSWP3v1">
CLMGSWP3v1.Solar,CLMGSWP3v1.Precip,CLMGSWP3v1.TPQW
</value>
This should be expanded to have an option for GSWP3 forcing and SSP compsets. So something like:
<value datm_mode="CLMGSWP3v1" compset_period="SSP585">
CLMGSWP3v1.Solar,CLMGSWP3v1.Precip,CLMGSWP3v1.TPQW,Anomaly.Forcing.cmip6.ssp585
</value>
There might be a way to append just the AF file to the end, which would be preferred. But, we'd need to figure that out. The matching needs to be figured out to make sure it works right. And of course all the SSP options need to be added.
Note that compset_period needs to be added to the buildnml file which I'll sketch out below.
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.
Changes to buildnml:
buildnml needs to:
- Get the compset from the case
- Extract out the period part of the compset
- Send compset_period to the config object
So something like...
caseroot = case.get_value("CASEROOT")
datm_mode = case.get_value("DATM_MODE")
datm_topo = case.get_value("DATM_TOPO")
compset = case.get_value("COMPSET")
.
.
.
# Do some reg-ex or other magic to get the compset_period from it
compset_period = something(compset)
config['datm_mode'] = datm_mode
config['datm_co2_tseries'] = datm_co2_tseries
config['compset_period'] compset_period
.
.
.
This reverts commit 3f36622b3bd6358d324f1aca5e90009db306963e, which tried to do this in namelist_definition_datm.xml, and instead does it in datm/cime_config/buildnml. Only looks at compset if anomaly_forcing not specified in namelist.
…rounding quote marks.
Re-requesting review from @ekluzek, because it seems like I've already addressed his concern. If approved, I'll continue by merging in the latest CTSM tag into ESCOMP/CTSM#2686 and the corresponding CDEPS tag here. |
Description of changes
Fixes a bug, and also makes it much simpler to run land-only SSP cases.
Specific notes
Contributors other than yourself, if any: @ekluzek
CDEPS Issues Fixed (include github issue #):
Are there dependencies on other component PRs (if so list): This doesn't depend on other components. However, I will soon submit a PR to CTSM that depends on this branch. (See issue ESCOMP/CTSM#2301.)
Are changes expected to change answers (bfb, different to roundoff, more substantial): Substantial, but only if people were being bitten by #258 before.
Any User Interface Changes (namelist or namelist defaults changes): Yes.
Adds
anomaly_forcing
options:Anomaly.Forcing.cmip5.rcp45
Anomaly.Forcing.cmip6.ssp126
Anomaly.Forcing.cmip6.ssp245
Anomaly.Forcing.cmip6.ssp370
Anomaly.Forcing.cmip6.ssp585
Removes all variable-specific
anomaly_forcing
options.Testing performed (e.g. aux_cdeps, CESM prealpha, etc):
datm_ssp126_anom_forc
, is bit-for-bit identical to previous version (ctsm5.2.015
).aux_cdeps
test of SSP585 compset, added the 3 other SSP compsets. Allaux_cdeps
SSP tests show diffs as expected fromcdeps1.0.34
.Hashes used for testing:
Remaining work
anomaly_forcing
is automatically set for eachISSP
compsetI
SSP
compset to test) Ensure that it's only set forDATM
compsetsanomaly_forcing
in the generateddatm_in
: Fix that. (Note that the anomaly forcing file IS correctly being put indatm.streams.xml
.)