Skip to content
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

Perform gridcell-level water balance checks bracketing the entire run loop #201

Closed
billsacks opened this issue Dec 29, 2017 · 8 comments
Closed
Assignees
Labels
enhancement new capability or improved behavior of existing capability

Comments

@billsacks
Copy link
Member

From @billsacks on March 17, 2017 20:38

Currently, the CLM driver loop looks like this, in terms of pieces relevant to this issue:

  • Compute pre-dynlu gridcell-level water mass

  • Adjust column areas via dynamic landunits

  • Compute post-dynlu gridcell-level water mass

  • Compute adjustment fluxes based on difference between pre- and post-dynlu gridcell-level water mass

  • Compute pre-physics column-level water mass

  • Do physics

  • Compute post-physics column-level water mass

  • Do column-level balance checks using the pre- and post-physics column-level water mass and the relevant fluxes

As long as you trust:

  1. that the calculation of the dynlu adjustment fluxes is done correctly, and

  2. that the column-level fluxes used in the column-level balance checks correctly mirror the relevant gridcell-level fluxes

then you can convince yourself that this sequence ensures gridcell-level conservation.

However, it would be more robust and more reassuring if we performed gridcell-level water conservation checks, bracketing essentially the entire driver run loop (including dynamic landunits adjustments as well as the physics). Ideally, the fluxes considered in these gridcell-level checks would be the actual fluxes received by and sent from CLM (gridcell-level precipitation, runoff, etc.). This last point is particularly important for runoff, which is now split into a few different terms, and it seems easy for some term to accidentally not be included in any of the runoff terms sent to the coupler - so we should do these gridcell-level balance checks using the runoff terms that are actually sent to the coupler.

The sequence would then look like:

  • Compute beginning gridcell-level water mass

  • Adjust column areas via dynamic landunits

  • Compute post-dynlu gridcell-level water mass

  • Compute adjustment fluxes based on difference between pre- and post-dynlu gridcell-level water mass

  • Compute pre-physics column-level water mass

  • Do physics

  • Compute post-physics column-level water mass; also average this to the gridcell level

  • Do column-level balance checks using the pre- and post-physics column-level water mass and the relevant fluxes

  • Do gridcell-level balance checks using the beginning gridcell-level water mass and ending gridcell-level water mass. For this check, ideally use the gridcell-level fluxes that are passed to and from CLM

Note that this plan still maintains the column-level balance checks: These are useful for seeing which column(s) have balance errors. But it's possible that we could avoid doing these in general, and just drill down to the column-level if a gridcell-level balance check fails.

Copied from original issue: NCAR/CLM#3

@billsacks
Copy link
Member Author

billsacks commented Dec 29, 2017

From @billsacks on March 17, 2017

One thing needed in counting up the gridcell-level water mass is the pseudo-state from the annual dribblers (i.e., the result of get_amount_left_to_dribble): treat this like any other state for purposes of the conservation check (i.e., add it to the initial state and to the final state)

@billsacks billsacks added the enhancement new capability or improved behavior of existing capability label Dec 29, 2017
@billsacks
Copy link
Member Author

billsacks commented Dec 29, 2017

From @billsacks on April 28, 2017

Note that, with the change in clm4_5_15_r235 (addition of for_testing_zero_dynbal_fluxes namelist flag), it's possible that we sometimes won't conserve gridcell-level water and energy (if this flag is set). We'll need to handle this case, probably by introducing a term to track the lack of conservation, and including that term in the balance checks.

@billsacks
Copy link
Member Author

Another thing that I think the current balance checks are missing is a check of the downscaling of atm2lnd terms, such as rain-snow repartitioning. The balance checks should do checks using the pre-downscaled fluxes to ensure that we're not gaining or losing mass in the downscaling code. (This may be relatively independent of the rest of this issue.)

billsacks added a commit to billsacks/ctsm that referenced this issue Dec 5, 2018
Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)
billsacks added a commit that referenced this issue Dec 6, 2018
Option for rain-to-snow to immediately run off in some regions

Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
#201 (comment) .)

CTSM Master Tag This Corresponds To: N/A

   At least for now, we are bringing this to the release branch but not
   to master. Here is an excerpt from the email explaining this
   rationale:

   My question is: Should I do this:

   (1) Just on a branch off of the release-clm5.0 branch, with no plan
       to bring it back to release-clm5.0 or master

   (2) On the release-clm5.0 branch, but not bring it back to master

   (3) On the release-clm5.0 branch and master

   My inclination right now is towards (2). I don't really like (1)
   because this change will be wanted for a number of CMIP6-related
   experiments, and it feels like it could be a pain to keep this branch
   up-to-date with the evolving release-clm5.0 branch. However, the
   changes are going to be a bit messy and having this be
   region-specific isn't really physically justifiable (it's just being
   done that way to keep the climate as close as possible to the
   official CMIP6 runs), so I'm not sure we really want this on
   master. If we did anything on master, I could imagine having a
   globally-applicable switch controlling this behavior, rather than
   having it apply to just certain glacier regions.

   Bette: The main reason I could see for bringing this to master is if
   you imagine needing to use this new option in isotope-enabled runs,
   since the isotope-enabled version of CTSM won't be on the
   release-clm5.0 branch. Do you think it's likely that you'd need to do
   that, or would the isotope-enabled runs use the standard CMIP6
   physics settings in this respect?
ekluzek added a commit to ekluzek/CTSM that referenced this issue Jan 14, 2019
Option for rain-to-snow to immediately run off in some regions

Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)

CTSM Master Tag This Corresponds To: N/A

   At least for now, we are bringing this to the release branch but not
   to master. Here is an excerpt from the email explaining this
   rationale:

   My question is: Should I do this:

   (1) Just on a branch off of the release-clm5.0 branch, with no plan
       to bring it back to release-clm5.0 or master

   (2) On the release-clm5.0 branch, but not bring it back to master

   (3) On the release-clm5.0 branch and master

   My inclination right now is towards (2). I don't really like (1)
   because this change will be wanted for a number of CMIP6-related
   experiments, and it feels like it could be a pain to keep this branch
   up-to-date with the evolving release-clm5.0 branch. However, the
   changes are going to be a bit messy and having this be
   region-specific isn't really physically justifiable (it's just being
   done that way to keep the climate as close as possible to the
   official CMIP6 runs), so I'm not sure we really want this on
   master. If we did anything on master, I could imagine having a
   globally-applicable switch controlling this behavior, rather than
   having it apply to just certain glacier regions.

   Bette: The main reason I could see for bringing this to master is if
   you imagine needing to use this new option in isotope-enabled runs,
   since the isotope-enabled version of CTSM won't be on the
   release-clm5.0 branch. Do you think it's likely that you'd need to do
   that, or would the isotope-enabled runs use the standard CMIP6
   physics settings in this respect?
@billsacks billsacks added the priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations label Apr 17, 2019
@billsacks
Copy link
Member Author

Tagging this high priority along with #314 to avoid further problems like #675 .

@billsacks
Copy link
Member Author

It looks like this has been done in e3sm (maybe by @bishtgautam ), so that implementation could be used as a guide.

@bishtgautam
Copy link

A grid level water balance check have been added in ELM, but I believe that balance check only works for the case when dyn subgrid driver isn't active.

@billsacks
Copy link
Member Author

Thanks, @bishtgautam !

Also, a suggestion from @dlawrenncar - If the performance cost of the column-level checks is significant, we could do something like: Initialize both the gridcell and column-level balance checks. Then, at the end of the time step, just do the gridcell-level check; only do the column-level check if this gridcell-level check fails. The rationale is: the gridcell-level check should probably be sufficient to catch balance errors; the main point of the column-level check would then be to give additional information about the column with the problem.

Whether or not we do that, I realized that: We should either do the column-level checks first or, if we do the gridcell-level checks first (e.g., as per @dlawrenncar 's suggestion), then we should wait to abort until after we've done the column-level checks, so that we print information from the column-level checks.

@billsacks
Copy link
Member Author

Referring to the last comment (#201 (comment)): I think it's possible that column-level balance checks would fail even if the gridcell-level checks pass. However, this would often (always?) be due to a problem in the balance check itself, such as forgetting to add a flux in the check. This makes me think: If we go with Dave Lawrence's suggestion of only doing the column-level checks if the gridcell-level checks have failed, then we should probably additionally do the column-level checks always in DEBUG mode (using #ifndef NDEBUG), to catch code errors in the column-level balance checks.

@billsacks billsacks removed the priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations label Feb 6, 2020
ekluzek added a commit that referenced this issue Dec 16, 2023
0f884bfec Merge pull request #205 from jedwards4b/sunset_svn_git_access
82a5edf79 merge in billsacks:svn_testing_no_github
17532c160 Use a local svn repo for testing
9c904341a different method to determine if in tests
539952ebd remove debug print statement
cc5434fa7 fix submodule testing
1d7f28840 remove broken tests
04e94a519 provide a meaningful error message
38bcc0a8c Merge pull request #201 from jedwards4b/partial_match
b4466a5aa remove debug print statement
c3cf3ec35 fix issue with partial branch match
7b6d92ef6 Merge pull request #198 from johnpaulalex/gitdir
927ce3a98 Merge pull request #197 from johnpaulalex/testpath
a04f1148f Merge pull request #196 from johnpaulalex/readmod
d9c14bf25 Change the rest of the methods to use -C. Still some usage of getcwd in test_unit_repository_git.
332b10640 Fix incorrect logged path of checkout_externals in test_sys_checkout: it was basically the parent of the current directory, which varies throughout the test. (it called abspath with '{0}/../../', which adds arbitrary and not-interpolated subdir '{0}' to the path, then removes it and removes one more level).
932a7499b Remove printlog from read_gitmodules_file since read_externals_description_file() already has a nearly-the-same printlog (but add it to the other caller).
5d13719ed Merge pull request #195 from johnpaulalex/check_repo
423395449 Update utest to mock _git_remote_verbose in a new way, since it is now called via the GitRepository class rather than on the specific GitRepository instance.
d7a42ae96 Check that desired repo was actually checked out.
71596bbc1 Merge pull request #194 from johnpaulalex/manic2
4c96e824e Make the MANIC_TEST_BARE_REPO_ROOT env var special - give it a constant for easy tracking, and automatically tear it down after each test.
259bfc04d test_sys_checkout: use actual paths in on-the-fly configs rather than MANIC_TEST_BARE_REPO_ROOT env var. This will make it easier to test (in the near future) that checkout_externals actually checked out the desired repo dir.
557bbd6eb Merge pull request #193 from johnpaulalex/manic
5314eede1 Remove MANIC_TEST_TMP_REPO_ROOT environment variable in favor of module-level variable.
345fc1e14 Merge pull request #191 from johnpaulalex/test_doc12
2117b843c test_sys_checkout: verify that basic by-tag/branch/hash tests actually take us to the correct git tag/branch/hash.
94d6e5f2b Merge pull request #190 from johnpaulalex/test_doc11
3ff33a6a8 Inline local-path-creation methods
47dea7f64 Merge pull request #189 from johnpaulalex/test_doc10
9ea75cbf8 Grab-bag of renamings: Remove redundant _NAME from repo constants, and consistently add _REPO suffix (This causes the majority of diffs).
c0c847ec8 Merge pull request #188 from johnpaulalex/test_doc9
2dd5ce0f7 test_sys_checkout.py: only check for correct 'required' or 'optional' state in the test that exercises required vs optional behavior. Removed a lot of boilerplate.
eb3085984 Merge pull request #187 from johnpaulalex/test_doc8
1832e1f84 test_sys_checkout: Simplify many tests to only use a single external.
8689d61ec Merge pull request #186 from johnpaulalex/test_doc7
fbee4253e Grab bag of test_sys_checkout cleanups:    Doc inside of each test more clearly/consistently.    TestSysCheckoutSVN didn’t get the inlining-of-helper-methods treatment, now it has that.    Move various standalone repo helper methods (like create_branch) into a RepoUtils class.    README.md was missing newlines when rendered as markdown.    Doc the return value of checkout.main    Fix test_container_exclude_component - it was looking for the wrong key (which is never present); now it looks for the correct key.
f0ed44a6e Merge pull request #185 from johnpaulalex/test_doc6
a3d59f5f2 Merge pull request #184 from johnpaulalex/test_doc5
5329c8ba7 test_sys_checkout: Inline config generation functions that are only called once.
464f2c7a7 test_sys_checkout: Inline another layer (per-config-file checks). Rename the 4 methods that are used multiple times, to reflect what they do rather than what they're called.
8872c0df6 Merge pull request #183 from johnpaulalex/doc_test4
c045335f6 Merge pull request #182 from johnpaulalex/doc_test3
c583b956e Merge pull request #181 from johnpaulalex/doc_test2
e01cfe278 test_sys_checkout: less confusing handling of return values from checkout_externals. Specifically, when doing a checkout, don't return tree_status from _before_ the checkout. Make a new wrapper to call checkout_externals a second time, to calculate the new status after a checkout (very frequent pattern).
23286818c test_sys_checkout: Remove another layer (which generates test component names)
c3717b6bc Merge pull request #180 from johnpaulalex/doc_test
36d7a4434 test_sys_checkout.py: remove one layer of functions (that check for local status enums). No-op.
2c4584bf7 More documentation about tests: * contents of test repositories (n a new README.md) * various constants in test_sys_checkout.py that point to those contents, and terminology like container/simple/mixed. * in each test method, the scenarios being tested. * The coupling between test methods.
55e74bd0a Merge pull request #179 from johnpaulalex/circ
66be84290 Remove circular dependency by making _External stop doing tricky things with sourcetrees.
82d3b247f Merge pull request #178 from johnpaulalex/test_doc
3223f49ea Additional documentation of system tests - global variables, method descriptions.
45b7c01c3 Merge pull request #177 from jedwards4b/git_workflow
ace90b2c2 try setting credentials this way
f4d6aa933 try setting credentials this way
1d61a6944 use this to set git credentials
7f9d330e1 use this to set git credentials
5ac731b85 add tmate code
836847be7 get git workflow working
dcd462d71 Merge pull request #176 from jedwards4b/add_github_testing
2d2479e9d Merge pull request #175 from johnpaulalex/fix
711a53fdf add github testing of prs and automatic tagging of main
cfe0f888a fix typos
5665d6140 Fix broken checkout behavior introduced by PR #172.
27909e255 Merge pull request #173 from johnpaulalex/readall
00ad0440b Further tiny refactorings and docs of checkout API (no-op).    Remove unused load_all param in _External.checkout().    Rename _External.checkout_externals() to checkout_subexternals(), to remove the ambiguity about whether the main external pointed to by the _External is itelf checked out (it is not)    Clarify load_all documentation - it’s always recursive, but applies different criteria at each level.    Rename variables in checkout.py (e.g. ext_description)  to match the equivalent code in sourcetree.py.
2ea3d1a3a Merge pull request #172 from johnpaulalex/fixit
43bf8092c Merge pull request #171 from johnpaulalex/docstatus
e6aa7d21e Merge pull request #170 from johnpaulalex/printdir
adbd71557 On checkout, refresh locally installed optional packages regardless of whether -o is passed in.
add074593 Comment tweaks, and fix 'ppath' typo
696527cb8 Document the format of various status dictionaries, and the various paths and path components within an _External.
c677b9403 When processing an external, print out its path in addition to the base filename (to disambiguate all the externals.cfg's)
975d7fd5a Merge pull request #169 from johnpaulalex/docfix_branch
09709e36d Document _Externals.status().  The original comment was apparently copy-pasted from checkout().
1d880e090 Merge pull request #167 from billsacks/fix_svn_on_windows
3510da848 Tweak a unit test to improve coverage
eb7fc1368 Handle the possibility that the URL already ends with '/'
02ea87e3d Fix svn URLs on Windows
b1c02ab54 Merge pull request #165 from gold2718/doc_fix
9f4be8c7b Add documentation about externals = None feature
a3b3a0373 Merge pull request #162 from ESMCI/fischer/python3
d4f1b1e8d Change shebang lines to python3
2fd941abc Merge pull request #158 from billsacks/modified_solution
de08dc2ee Add another option for when an external is in a modified state
e954582d0 Merge pull request #156 from billsacks/onbranch_show_hash
952e44d51 Change output: put tag/hash before branch name
10288430f Fix pre-existing pylint issues
01b13f78f When on a branch, show tag/hash, too
39ad53263 Merge pull request #150 from gold2718/fix_combo_config
75f8f02f5 Merge pull request #152 from jedwards4b/sort_by_local_path
42687bd53 remove commented code
29e26af81 fix pylint issues
7c9f3c613 add a test for nested repo checkout
75c5353d2 fix spacing
24a3726a1 improve sorting, checkout externals with each comp
29f45b086 remove py2 test and fix super call
880a4e765 remove decode
1c53be854 no need for set call
36c56dbac simplier fix for issue
dc67cc682 simpler solution
b32c6fca9 fix to allow submodule name different from path
5b5e1c2b0 Merge pull request #144 from billsacks/improve_errmsg
c983863c4 Add another option for dealing with modified externals
59ce252cf Add some details to the error message when externals are modified
be5a1a4d7 Merge pull request #143 from jedwards4b/add_exclude
2aa014a1b fix lint issue
49cd5e890 fix lint issues
418173ffd Added tests for ExternalsDescriptionDict
afab352c8 fix lint issue
be85b7d1b fix the test
a580a570b push test
d43710864 add a test
21affe33c fix formatting issue
72e6b64ae add an exclude option

git-subtree-dir: manage_externals
git-subtree-split: 0f884bfec8e43d0c02261de858d6ec3f6d855e51
ekluzek added a commit to dmleung/CTSM that referenced this issue Jan 26, 2024
0f884bfec Merge pull request ESCOMP#205 from jedwards4b/sunset_svn_git_access
82a5edf79 merge in billsacks:svn_testing_no_github
17532c160 Use a local svn repo for testing
9c904341a different method to determine if in tests
539952ebd remove debug print statement
cc5434fa7 fix submodule testing
1d7f28840 remove broken tests
04e94a519 provide a meaningful error message
38bcc0a8c Merge pull request ESCOMP#201 from jedwards4b/partial_match
b4466a5aa remove debug print statement
c3cf3ec35 fix issue with partial branch match

git-subtree-dir: manage_externals
git-subtree-split: 0f884bfec8e43d0c02261de858d6ec3f6d855e51
ekluzek added a commit to ekluzek/CTSM that referenced this issue Feb 21, 2024
0f884bfec Merge pull request ESCOMP#205 from jedwards4b/sunset_svn_git_access
82a5edf79 merge in billsacks:svn_testing_no_github
17532c160 Use a local svn repo for testing
9c904341a different method to determine if in tests
539952ebd remove debug print statement
cc5434fa7 fix submodule testing
1d7f28840 remove broken tests
04e94a519 provide a meaningful error message
38bcc0a8c Merge pull request ESCOMP#201 from jedwards4b/partial_match
b4466a5aa remove debug print statement
c3cf3ec35 fix issue with partial branch match

git-subtree-dir: manage_externals
git-subtree-split: 0f884bfec8e43d0c02261de858d6ec3f6d855e51
samsrabin pushed a commit to samsrabin/CTSM that referenced this issue May 3, 2024
Reusable workflow

Description of changes

Create reusable workflows and stop duplicating code
Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial):

Any User Interface Changes (namelist or namelist defaults changes):

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):

Hashes used for testing:
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this issue Jun 27, 2024
Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this issue Jun 27, 2024
Option for rain-to-snow to immediately run off in some regions

Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)

CTSM Master Tag This Corresponds To: N/A

   At least for now, we are bringing this to the release branch but not
   to master. Here is an excerpt from the email explaining this
   rationale:

   My question is: Should I do this:

   (1) Just on a branch off of the release-clm5.0 branch, with no plan
       to bring it back to release-clm5.0 or master

   (2) On the release-clm5.0 branch, but not bring it back to master

   (3) On the release-clm5.0 branch and master

   My inclination right now is towards (2). I don't really like (1)
   because this change will be wanted for a number of CMIP6-related
   experiments, and it feels like it could be a pain to keep this branch
   up-to-date with the evolving release-clm5.0 branch. However, the
   changes are going to be a bit messy and having this be
   region-specific isn't really physically justifiable (it's just being
   done that way to keep the climate as close as possible to the
   official CMIP6 runs), so I'm not sure we really want this on
   master. If we did anything on master, I could imagine having a
   globally-applicable switch controlling this behavior, rather than
   having it apply to just certain glacier regions.

   Bette: The main reason I could see for bringing this to master is if
   you imagine needing to use this new option in isotope-enabled runs,
   since the isotope-enabled version of CTSM won't be on the
   release-clm5.0 branch. Do you think it's likely that you'd need to do
   that, or would the isotope-enabled runs use the standard CMIP6
   physics settings in this respect?
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this issue Jul 5, 2024
Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this issue Jul 5, 2024
Option for rain-to-snow to immediately run off in some regions

Up until now: When repartition_rain_snow is .true. (which is the default
for CLM5), rain that falls when the near-surface temperature is cold is
converted to snow. This repartitioning was put in place for two
reasons: (1) Downscaling to elevation classes: changing the balance
between rain and snow for different elevation classes; (2) Correcting
problems in CAM. However, members of the Land Ice Working Group would
like to change this behavior so that, when CAM produces cold-temperature
rain, this rain immediately runs off rather than being converted to
snow. The purpose of this is to reduce the too-high SMB over portions of
Greenland in CESM2 coupled runs (which results in part from CAM's
generation of liquid precipitation despite very cold temperatures).

This new behavior is implemented in a glacier region-specific manner,
based on a new namelist flag, glacier_region_rain_to_snow_behavior. It
is not at all ideal to make this aspect of the physics differ by region,
but this has been requested by members of the Land Ice Working Group in
order to address biases over Greenland while having minimal impact on
the climate (so that the climate can stay very similar to that of the
official CMIP6 runs). Note that, unlike other glacier region-specific
behaviors, this one applies to all landunits, not just glaciers. This
also seems a bit non-ideal, but we want the physics to be the same for
all landunit types in a given region, and we also want this behavior to
apply to vegetated columns because they are used for glacial
inception (and we want this alternate behavior to apply to glacial
inception, too, in order to decrease some instances of inception).

The justification for this new physics is: In the case of (1) above: If
CAM is generating rain at a given elevation / temperature, that doesn't
necessarily imply that an equal water equivalent of snow would be
generated at a higher elevation / lower temperature: indeed, in reality,
there might not be any precipitation falling at that higher elevation /
lower temperature. In the case of (2) above: There seem to be problems
with CAM's microphysics that cause it to produce too much rain when
temperatures are very cold; it seems (at least to some people) equally
justifiable to throw this cold rain away (by sending it to the ocean as
runoff) as it is to convert this cold rain to snow.

Note: I don't think any changes are needed in
BalanceCheck (unfortunately), since BalanceCheck currently uses the
post-downscaling precipitation fluxes, and the pre-lnd2atm runoff
fluxes (i.e., the new runoff flux isn't included in the terms in
BalanceCheck, and it doesn't need to be because BalanceCheck uses the
post-downscaling precipitation fluxes). (See also
ESCOMP#201 (comment) .)

CTSM Master Tag This Corresponds To: N/A

   At least for now, we are bringing this to the release branch but not
   to master. Here is an excerpt from the email explaining this
   rationale:

   My question is: Should I do this:

   (1) Just on a branch off of the release-clm5.0 branch, with no plan
       to bring it back to release-clm5.0 or master

   (2) On the release-clm5.0 branch, but not bring it back to master

   (3) On the release-clm5.0 branch and master

   My inclination right now is towards (2). I don't really like (1)
   because this change will be wanted for a number of CMIP6-related
   experiments, and it feels like it could be a pain to keep this branch
   up-to-date with the evolving release-clm5.0 branch. However, the
   changes are going to be a bit messy and having this be
   region-specific isn't really physically justifiable (it's just being
   done that way to keep the climate as close as possible to the
   official CMIP6 runs), so I'm not sure we really want this on
   master. If we did anything on master, I could imagine having a
   globally-applicable switch controlling this behavior, rather than
   having it apply to just certain glacier regions.

   Bette: The main reason I could see for bringing this to master is if
   you imagine needing to use this new option in isotope-enabled runs,
   since the isotope-enabled version of CTSM won't be on the
   release-clm5.0 branch. Do you think it's likely that you'd need to do
   that, or would the isotope-enabled runs use the standard CMIP6
   physics settings in this respect?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability
Projects
None yet
Development

No branches or pull requests

3 participants