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

Remove infiltration excess runoff for VIC #190

Merged
merged 4 commits into from
Dec 28, 2017

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Dec 28, 2017

NOTE: This PR was originally created in the temporary CTSM repository, Sep 13, 2017, where it was NCAR/clm-ctsm#17. I am moving over most of the comments from there: both my own and those of @martynpclark .

Here was the original comment:

Martyn Clark reviewed the VIC implementation, and felt that the current
implementation of infiltration excess runoff is inconsistent with the
standard VIC implementation. It appears that what was being called VIC's
infiltration excess runoff was actually just an attempt to give a better
numerical approximation to the solution for saturated surface excess
runoff. So deleting this leaves only a first-order approximation to
VIC's saturated surface excess runoff.

Eventually we may want to put in place a more accurate solution for
VIC's saturated surface excess runoff. But Martyn's feeling is that this
can come in with other changes we want to make regarding numerical
solutions in CTSM.

Martyn Clark reviewed the VIC implementation, and felt that the current
implementation of infiltration excess runoff is inconsistent with the
standard VIC implementation. It appears that what was being called VIC's
infiltration excess runoff was actually just an attempt to give a better
numerical approximation to the solution for saturated surface excess
runoff. So deleting this leaves only a first-order approximation to
VIC's saturated surface excess runoff.

Eventually we may want to put in place a more accurate solution for
VIC's saturated surface excess runoff. But Martyn's feeling is that this
can come in with other changes we want to make regarding numerical
solutions in CTSM.
!
! 1e200 mm H2O /s seems large enough to be effectively infinite, while not being so
! large as to cause floating point overflows elsewhere.
real(r8), parameter :: qinmax_unlimited = 1.e200_r8 ! mm H2O /s
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @billsacks Sep 13, 2017

When we talked about this a few weeks ago, there were suggestions of using a global constant here.

Reasons I went with a locally-defined qinmax_huge rather than a globally-defined constant:

  • Globally-defined is useful to eliminate the need for, "If I change this here, then I also need to change this over there". That doesn't seem to be the case here:
    • Nobody needs to check this
    • I couldn't see any benefits to having this shared between different parameterizations
  • Problems with globally-defined
    • A different value might make more sense in other contexts
    • If another parameterization needs a different value and so changes the global constant, it would change this and other previous parameterizations that use it, which could inadvertently change behavior

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @martynpclark Sep 13, 2017

Agreed. This seems like a good approach.

@billsacks
Copy link
Member Author

Comment from @martynpclark Sep 13, 2017

@billsacks -- good description -- the VIC implementation was actually the analytical solution to saturation-excess runoff (which depends on saturated area), not infiltration-excess runoff (which depends on the flux at the upper boundary).

@billsacks
Copy link
Member Author

Comment from @martynpclark Sep 13, 2017

@billsacks -- we have already got to the point where we are making changes to the numerical solutions in CTSM. The VIC implementation provides an analytical solution, in the same way that we use analytical solutions for the ponded water store.

@billsacks
Copy link
Member Author

Comment from @martynpclark Sep 13, 2017

The code for the VIC analytical solution is by no means obvious. The derivation is in this pdf file:

VIC_surfaceRunoff.pdf

rsurf_vic = (qflx_in_soil(c)*dtime - top_max_moist(c) + top_moist(c) &
+ top_max_moist(c) * basis**(1._r8 + b_infil(c)))/dtime
end if
rsurf_vic = min(qflx_in_soil(c), rsurf_vic)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @martynpclark Sep 13, 2017

It is correct to delete this code here, and now that we have options for analytical numerical solutions, the code block below can be moved to the saturation excess runoff routine:

       max_infil = (1._r8+b_infil(c)) * top_max_moist(c)
       i_0 = max_infil * (1._r8 - (1._r8 - fsat(c))**(1._r8/b_infil(c)))
       if(qflx_in_soil(c) <= 0._r8) then
          rsurf_vic = 0._r8
       else if(max_infil <= 0._r8) then
          rsurf_vic = qflx_in_soil(c)
       else if((i_0 + qflx_in_soil(c)*dtime) > max_infil) then             !(Eq.(3a) Wood et al. 1992)
          rsurf_vic = (qflx_in_soil(c)*dtime - top_max_moist(c) + top_moist(c))/dtime
       else                                                                !(Eq.(3b) Wood et al. 1992)
          basis = 1._r8 - (i_0 + qflx_in_soil(c)*dtime)/max_infil
          rsurf_vic = (qflx_in_soil(c)*dtime - top_max_moist(c) + top_moist(c)    &
               + top_max_moist(c) * basis**(1._r8 + b_infil(c)))/dtime
       end if

We can use the same logic as in the ponded water (options for explicit Euler [current code] and the analytical solution [this code]).

Again, the pdf document above provides derivations.

! This implementation gives a first-order approximation to saturated excess runoff.
! For now we're not including the more exact analytical solution, or even a better
! numerical approximation.
!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from @martynpclark Sep 13, 2017

We can add the analytical solution here.

@billsacks
Copy link
Member Author

Comment from @billsacks Sep 14, 2017

@martynpclark I feel like there are still two somewhat unresolved issues, both really boiling down to how much complexity we want to support:

  1. I recall that moving this code into the saturated surface excess runoff calculation would mean that different parameterizations for saturated surface excess runoff now potentially differ in more than just fsat. That's fine in principle, but my sense from the Aug 24 ctsm meeting was that nobody was very happy with what that would mean in terms of code complexity, and that we only wanted to go there if we really had to. It's unclear to me whether the vic option is important enough to justify this extra complexity itself, or if this is only worth doing if some other parameterizations also require this.

  2. I feel like we haven't totally worked out how we want to select between different solution methods. In this case, I'd personally be more inclined to just always include the more exact analytical solution, rather than providing an option to include it or exclude it, in order to avoid extra levels of complexity. Of course, I'm open to having this be an option, but I feel we should be careful to only include options like this where they're really warranted.

We can continue this discussion here if you want. Or we can defer this discussion to a later date if (as is my impression) this vic stuff isn't the highest priority.

@billsacks
Copy link
Member Author

Comment from @martynpclark Sep 14, 2017

Hi @billsacks -- I agree. Let's defer the decision until once we converge on the overall structure that we are happy with. Yes, I agree for the operator splitting implementation that we should (ideally) always use the more accurate method if it is available -- one question is if there is a desire to have what was present before, since different solutions can lead to different model behavior.

@billsacks
Copy link
Member Author

Comment from @billsacks Sep 14, 2017

@martynpclark

I agree. Let's defer the decision until once we converge on the overall structure that we are happy with.

So any feelings about what I should do in the meantime?:

  1. Merge this PR as is (once I have tested it)

  2. Hold off on merging this PR until we decide on the final solution

one question is if there is a desire to have what was present before, since different solutions can lead to different model behavior.

But here, isn't the more accurate method what was there before? (Just now we'd be applying it to saturated surface excess runoff rather than infiltration excess runoff?)

@billsacks
Copy link
Member Author

Comment from @martynpclark Sep 14, 2017

Hi @billsacks

So any feelings about what I should do in the meantime?

My preference is to merge this PR as is

But here, isn't the more accurate method what was there before? (Just now we'd be applying it to saturated surface excess runoff rather than infiltration excess runoff?)

Exactly. My comments were intended to be more general. There may be cases where we add a more accurate solution (as in the case for ponded water), and we still may have the desire to retain what was present before (since different solutions can lead to different model behavior). In these cases we may not always want to use the more accurate solution, and we retain capabilities in order to compare against previous model versions.

@billsacks
Copy link
Member Author

Comment from @billsacks Sep 14, 2017

@martynpclark thanks for the clarifications - this all makes sense now. I'll run testing on this as soon as I'm done with #13

@billsacks billsacks merged commit 5f43763 into unified_land_model Dec 28, 2017
billsacks added a commit that referenced this pull request Dec 28, 2017
(This has been copied to pull request #190 from
ESCOMP/vic_no_inf_excess_runoff)

Remove infiltration excess runoff for VIC

Martyn Clark reviewed the VIC implementation, and felt that the current
implementation of infiltration excess runoff is inconsistent with the
standard VIC implementation. It appears that what was being called VIC's
infiltration excess runoff was actually just an attempt to give a better
numerical approximation to the solution for saturated surface excess
runoff. So deleting this leaves only a first-order approximation to
VIC's saturated surface excess runoff.

Eventually we may want to put in place a more accurate solution for
VIC's saturated surface excess runoff. But Martyn's feeling is that this
can come in with other changes we want to make regarding numerical
solutions in CTSM.
@billsacks billsacks deleted the vic_no_infl_excess_runoff branch December 28, 2017 20:25
@billsacks
Copy link
Member Author

@martynpclark in case it wasn't clear: no action / replies are needed; I just wanted to move over our old conversation to save it in the new repository.

slevis-lmwg added a commit to slevis-lmwg/ctsm that referenced this pull request Nov 8, 2021
ERP_D_P36x2_Ld3.f10_f10_mg37.I2000Clm51BgcCrop.cheyenne_gnu.clm-mimics
SMS_Ld5_Mmpi-serial.1x1_brazil.IHistClm50BgcQianRs.izumi_gnu.clm-mimics
For the record the same cheyenne test using the intel compiler triggers
this error:
25:MPT ERROR: Rank 25(g:25) received signal SIGFPE(8).
25:     Process ID: 24080, Host: r14i4n18, Program: /glade/scratch/slevis/ERP_D_P36x2_Ld3.f10_f10_mg37.I2000Clm51BgcCrop.cheyenne_intel.clm-mimics.20211108_123526_stdha2/bld/cesm.exe
25:     MPT Version: HPE MPT 2.22  03/31/20 15:59:10
25:
25:MPT: --------stack traceback-------
25:OMP: Warning ESCOMP#190: Forking a process while a parallel region is active is potentially unsafe.
-1:MPT ERROR: MPI_COMM_WORLD rank 29 has terminated without calling MPI_Finalize()
-1:     aborting job
MPT: Received signal 8
billsacks added a commit that referenced this pull request Mar 3, 2023
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.

git-subtree-dir: manage_externals
git-subtree-split: 7b6d92ef689e2f65733e27f8635ab91fb341356b
ekluzek added a commit that referenced this pull request 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
samsrabin pushed a commit to samsrabin/CTSM that referenced this pull request May 3, 2024
correct mismatch of leap year
### Description of changes
Handle a number of special cases in matching inputdata dates to model dates.
These cases are:

-   Model is no_leap, data is Gregorian and leapyear date 2/29 is encountered in data - skip date
-   Model is Gregorian, data is no_leap and leapyear date 2/29 is encountered in model - repeat 2/28 data
-   Model is Gregorian, data is gregorian but leapyears do not align.
-       if in model leap year repeat data from 2/28 
-       if in data leap year skip date 2/29


### Specific notes

Contributors other than yourself, if any:  @mvertens 

CDEPS Issues Fixed (include github issue #): ESCOMP#190 

Are there dependencies on other component PRs (if so list): ESCOMP/CESM_share#35

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

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

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):  Neon spinup of over 100 years

Hashes used for testing:

sM  ./ccs_config
        modified sandbox, ccs_config_cesm0.0.44 (branch main) --> ccs_config_cesm0.0.38
    ./cime
        clean sandbox, on cime6.0.45
s   ./components/cdeps
        clean sandbox, 0f3f707 (branch leap_year_corrections) --> cdeps0.12.63
    ./components/cdeps/fox
        clean sandbox, on 4.1.2.1
    ./components/cdeps/share/genf90
        clean sandbox, on genf90_200608
    ./components/cism
        clean sandbox, on cismwrap_2_1_95
    ./components/cism/source_cism
        clean sandbox, on cism_main_2.01.011
 M  ./components/cmeps
        modified sandbox, on cmeps0.13.71
    ./components/cpl7
        clean sandbox, on cpl7.0.14
    ./components/mizuRoute
        clean sandbox, on 34723c2e4df7caa16812770f8d53ebc83fa22360
    ./components/mosart
        clean sandbox, on mosart1_0_45
    ./components/rtm
        clean sandbox, on rtm1_0_78
e-o ./doc/doc-builder
        -, not checked out --> v1.0.8
    ./libraries/mct
        clean sandbox, on MCT_2.11.0
    ./libraries/parallelio
        clean sandbox, on pio2_5_7
s   ./share
        clean sandbox, bfa2b5d0a9de06153f2ac94a95818568a1f5cf11 (branch shr_cal_leapyear) --> share1.0.12
    ./src/fates
        clean sandbox, on sci.1.58.1_api.24.1.0
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this pull request Jun 27, 2024
(This has been copied to pull request ESCOMP#190 from
ESCOMP/vic_no_inf_excess_runoff)

Remove infiltration excess runoff for VIC

Martyn Clark reviewed the VIC implementation, and felt that the current
implementation of infiltration excess runoff is inconsistent with the
standard VIC implementation. It appears that what was being called VIC's
infiltration excess runoff was actually just an attempt to give a better
numerical approximation to the solution for saturated surface excess
runoff. So deleting this leaves only a first-order approximation to
VIC's saturated surface excess runoff.

Eventually we may want to put in place a more accurate solution for
VIC's saturated surface excess runoff. But Martyn's feeling is that this
can come in with other changes we want to make regarding numerical
solutions in CTSM.
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this pull request Jul 5, 2024
(This has been copied to pull request ESCOMP#190 from
ESCOMP/vic_no_inf_excess_runoff)

Remove infiltration excess runoff for VIC

Martyn Clark reviewed the VIC implementation, and felt that the current
implementation of infiltration excess runoff is inconsistent with the
standard VIC implementation. It appears that what was being called VIC's
infiltration excess runoff was actually just an attempt to give a better
numerical approximation to the solution for saturated surface excess
runoff. So deleting this leaves only a first-order approximation to
VIC's saturated surface excess runoff.

Eventually we may want to put in place a more accurate solution for
VIC's saturated surface excess runoff. But Martyn's feeling is that this
can come in with other changes we want to make regarding numerical
solutions in CTSM.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant