Fix and improve anomaly forcings for ISSP cases #292

samsrabin · 2024-08-09T03:29:43Z

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):

New CTSM testmods using these work correctly. Existing testmod, datm_ssp126_anom_forc, is bit-for-bit identical to previous version (ctsm5.2.015).
On top of existing aux_cdeps test of SSP585 compset, added the 3 other SSP compsets. All aux_cdeps SSP tests show diffs as expected from cdeps1.0.34.

Hashes used for testing:

Various throughout process.

Remaining work

Make it so that anomaly_forcing is automatically set for each ISSP compset
(Done but I need a non-I SSP compset to test) Ensure that it's only set for DATM compsets
If we care that it's not putting anomaly_forcing in the generated datm_in: Fix that. (Note that the anomaly forcing file IS correctly being put in datm.streams.xml.)

cdeps1.0.34

samsrabin · 2024-08-09T03:34:48Z

@ekluzek, a few questions:

What sort of changes/additions would you like me to make to aux_cdeps?
Does it make sense to keep the Anomaly.Forcing.Precip, Anomaly.Forcing.Temperature, etc. entries in stream_definition_datm.xml? If so, I'll need to rename them so they're specific to rcp45, then add equivalent ones for each SSP. Seems messy!
To make the ISSP compsets automatically use the right anomaly_forcing, would I do something like this in namelist_definition_datm.xml under <entry id="anomaly_forcing">?

<default_value>''</default_value>
<value compset="^SSP126_">Anomaly.Forcing.cmip6.ssp126</value>

jedwards4b · 2024-08-22T13:18:37Z

@samsrabin is this ready- if not can you make it a draft please.

jedwards4b

It's not clear to me that anomaly_forcing will always be defined and be an array.

datm/cime_config/buildnml

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.

samsrabin · 2024-09-05T20:14:16Z

Force-pushed to remove premature merge of main into this branch, which was interfering with my testing.

Resolves ESCOMP#310.

samsrabin · 2024-09-06T17:21:58Z

@ekluzek This is ready, if you'd like to review!

samsrabin · 2024-09-06T17:43:31Z

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.

samsrabin · 2024-11-04T17:25:59Z

@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.

jedwards4b · 2024-11-04T17:29:44Z

cime_config/stream_cdeps.py

@@ -336,14 +336,30 @@ def create_stream_xml(
                        ),
                    )
                    if var_key in valid_values:
+


If you are expecting that the variable may or may not have surrounding quotes why not just strip the quotes if they exist and avoid asking the user for a correction?

@samsrabin I think @jedwards4b is right here, it looks like there might be a simpler way to handle this. Could we get together and go over this together and see if there is a way to do that?

ekluzek

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.

ekluzek · 2024-11-04T23:49:12Z

cime_config/stream_cdeps.py

@@ -336,14 +336,30 @@ def create_stream_xml(
                        ),
                    )
                    if var_key in valid_values:
+


@samsrabin I think @jedwards4b is right here, it looks like there might be a simpler way to handle this. Could we get together and go over this together and see if there is a way to do that?

ekluzek · 2024-11-04T23:53:40Z

datm/cime_config/buildnml

+    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:


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.

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.

datm/datm_datamode_clmncep_mod.F90

datm/cime_config/stream_definition_datm.xml

ekluzek · 2024-11-05T18:35:10Z

datm/cime_config/stream_definition_datm.xml

-
-  <stream_entry name="Anomaly.Forcing.Precip">
+  
+    <stream_entry name="Anomaly.Forcing.cmip5.rcp45">


This does change the indentation, and it should go back to what it was before. "<stream" is the outermost level for types in the XML here.

ekluzek · 2024-11-05T19:04:09Z

datm/cime_config/namelist_definition_datm.xml

@@ -224,7 +224,7 @@
    <type>char(10)</type>


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.

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 . . .

samsrabin added 7 commits May 9, 2024 15:02

Add Anomaly.Forcing.cmip5.rcp45.

eff9a1a

Fix 'anomoly_forcing' typo.

e4bd386

Update meshfile used for Anomaly.Forcing.*.

7444e81

Fix more typos of "anomaly."

462c7a9

Add Anomaly.Forcing.cmip6.ssp126.

759c591

Add Anomaly.Forcing.cmip6.245, 370, 585.

84669d7

Merge tag 'cdeps1.0.34' into ssp-fix

5475066

cdeps1.0.34

samsrabin mentioned this pull request Aug 13, 2024

Fix and improve anomaly forcings for ISSP cases ESCOMP/CTSM#2686

Draft

6 tasks

jedwards4b approved these changes Aug 22, 2024

View reviewed changes

samsrabin marked this pull request as draft August 22, 2024 14:51

jedwards4b requested changes Sep 5, 2024

View reviewed changes

datm/cime_config/buildnml Outdated Show resolved Hide resolved

samsrabin added 7 commits September 5, 2024 14:12

anomaly_forcing now automatically set based on compset (ISSP*).

d2ba05b

SSP compsets now automatically get anomaly_forcing in datm_in.

82fde51

Remove Anomaly.Forcing.Precip, Anomaly.Forcing.Temperature, etc.

7927c1a

Set anomaly_forcing = 'none' to not use anomalies in an SSP compset.

cecc4a2

Only set anomaly_forcing for DATM SSP compsets.

e0b7594

Provide hint in error message if namelist variable invalid due to sur…

1e403a8

…rounding quote marks.

samsrabin force-pushed the ssp-fix branch from aeb13d7 to 1e403a8 Compare September 5, 2024 20:13

samsrabin added 2 commits September 5, 2024 15:15

Make datm buildnml more robust.

b1f1751

Bugfix to future-proofing of datm buildnml.

25c7b92

samsrabin mentioned this pull request Sep 6, 2024

Add aux_cdeps tests for SSPs other than 585 #310

Open

Add aux_cdeps tests for SSPs other than 585.

7cb5040

Resolves ESCOMP#310.

samsrabin marked this pull request as ready for review September 6, 2024 17:21

samsrabin requested a review from jedwards4b September 6, 2024 17:21

Merge branch 'main' into ssp-fix

a6b9560

jedwards4b requested changes Nov 4, 2024

View reviewed changes

ekluzek added bug Something isn't working enhancement New feature or request CESM Only Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group labels Nov 4, 2024

ekluzek requested changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix and improve anomaly forcings for ISSP cases #292

Fix and improve anomaly forcings for ISSP cases #292

samsrabin commented Aug 9, 2024 •

edited

Loading

samsrabin commented Aug 9, 2024 •

edited

Loading

jedwards4b commented Aug 22, 2024

jedwards4b left a comment

samsrabin commented Sep 5, 2024 •

edited

Loading

samsrabin commented Sep 6, 2024

samsrabin commented Sep 6, 2024

samsrabin commented Nov 4, 2024

jedwards4b Nov 4, 2024

ekluzek Nov 4, 2024

ekluzek left a comment

ekluzek Nov 4, 2024

ekluzek Nov 4, 2024

ekluzek Nov 5, 2024

ekluzek Nov 5, 2024

ekluzek Nov 5, 2024

ekluzek Nov 5, 2024


		<stream_entry name="Anomaly.Forcing.Precip">

		<stream_entry name="Anomaly.Forcing.cmip5.rcp45">

Fix and improve anomaly forcings for ISSP cases #292

Are you sure you want to change the base?

Fix and improve anomaly forcings for ISSP cases #292

Conversation

samsrabin commented Aug 9, 2024 • edited Loading

Description of changes

Specific notes

Remaining work

samsrabin commented Aug 9, 2024 • edited Loading

jedwards4b commented Aug 22, 2024

jedwards4b left a comment

Choose a reason for hiding this comment

samsrabin commented Sep 5, 2024 • edited Loading

samsrabin commented Sep 6, 2024

samsrabin commented Sep 6, 2024

samsrabin commented Nov 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekluzek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samsrabin commented Aug 9, 2024 •

edited

Loading

samsrabin commented Aug 9, 2024 •

edited

Loading

samsrabin commented Sep 5, 2024 •

edited

Loading