Skip to content

add again thresh_reset_absolute functionality - #19

Merged
fabern merged 13 commits into
mainfrom
add-again-thresh_terminate_absolute
Oct 28, 2026
Merged

fabern merged 13 commits into
mainfrom
add-again-thresh_terminate_absolute

Conversation

@fabern

@fabern fabern commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Ready to merge. The CPP file needs to implement it in a similar way as the it was when it was removed by commit: 1529a82

Intent: e.g. in the ModESim application thresh_terminate_absolute = 10 was used. (It is not needed for reproducability, since the cwd_global repo uses a pinned cwd version. But for future applications it might be useful to re-introduce it.

E.g. in the ModESim application `thresh_terminate_absolute = 10` was used. Reproducability is given since that repo used a pinned version of `cwd`. This commit also renames that argumnet as `thresh_reset_absolute` since it has the same effect as `doy_reset`.
@fabern

fabern commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

@copilot can you continue where I left off? Make a suggestion how to impelemnt the remaining # TODO in the cpp file. Consider the overall goal of re-establishing what was removed in commit 1529a82.

Co-authored-by: fabern <10245680+fabern@users.noreply.github.com>

Copilot AI commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

@copilot can you continue where I left off? Make a suggestion how to impelemnt the remaining # TODO in the cpp file. Consider the overall goal of re-establishing what was removed in commit https://g...

Implemented the reset as an independent event termination: once the running CWD is strictly below max_deficit - thresh_reset_absolute, the event ends and CWD resets. I also wired the numeric argument through R and Rcpp and added a regression test. Commit: d38155f. I couldn’t run the R tests here because Rscript is unavailable.

If we reset due to doy, always include last day in the instance (even if it might have a thresh_drop).

This simplifies code understanding, keeping all the reset behaviors close to each other.

Numeric changes are expected to be small, and only marginal case since `doy_reset` is expected to be set in moist periods.
…e_absolute`

The changes are mostly about length of the events, differing at most by 2 days.
Max deficit attained is reported the same.
@fabern
fabern marked this pull request as ready for review October 26, 2026 17:33
@fabern

fabern commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

From the NEWS.md:

# cwd (development version)
*  Added back previously removed arguments:
  * argument `thresh_reset_absolute` (i.e. absolute threshold-based event re-setting. 
  Resets CWD to zero and terminates event once deficit falls that amount below event peak.
  Same as previous `thresh_terminate_absolute`).
  * argument `thresh_reset_relative` (i.e. relative threshold-based event re-setting, 
  Resets CWD to zero and terminates event once deficit falls below a specified fraction of 
  the event peak. Same as previous `thresh_terminate_relative`).
* Fix: bug where length of deficit instance included day after drop below 0mm
* Breaking change: instances terminated by `doy_reset` now always include last day (even if 
  it would have a drop). Result change is expected to be very limited (off-by 1 day at most). 
  Moreover, this is expected to be an edge case, given that `doy_reset` is expected to be set 
  outside of periods with strong deficits.

@fabern
fabern merged commit 740656e into main Oct 28, 2026
6 checks passed
@fabern
fabern deleted the add-again-thresh_terminate_absolute branch October 28, 2026 15:22
@fabern

fabern commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

This is now merged and will be cwd v.3.0.3.

Doing the usual usethis-based workflow:

  • i) run usethis::use_version("patch"), check NEWS.md and push
  • ii) tag the release on GitHub by doing usethis::use_github_release()
  • iii) run usethis::use_dev_version() in the main branch so that further code changes in main during development are clearly marked as vX.X.X.9000.

Note, if step iii) is done too quickly after we pushed ii) then the pkgdown step might be cancelled. To have the documentation online one might need to restart that pkgdown step.

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.

2 participants