IDEMSInternational / R-Instat

A statistics software package powered by R
http://r-instat.org/
GNU General Public License v3.0
38 stars 102 forks source link

Water Balance Bug - Reducing with an Evaporation Variable given #8910

Closed lilyclements closed 2 weeks ago

lilyclements commented 3 months ago

@rdstern @MeSophie After a meeting with CIMH today, an issue was identified in the Water Balance code. This issue occurs when:

The issue is in the calculations for both wb_min and wb_max. The specific part of the code that needs correction is where these calculations are defined.

Originally, for wb_max, the code was set up to use purrr::accumulate2. We currently run this:

wb_max <- instat_calculation$new(type="calculation", function_exp="purrr::accumulate2(.f= ~ pmin(pmax(..1 + ..2, 0), 100), .y=tail(x=evapotranspiration, n=-1), .x=tail(x=rain_max, n=-1), .init=0)", result_name="wb_max", sub_calculations=list(rain_max))

We want to simplify this by switching from purrr::accumulate2 to purrr::accumulate. This combines the evapotranspiration and rain_max variables before applying the accumulation, thereby removing the need for a separate .y parameter. The corrected version for wb_max looks like this:

wb_max <- instat_calculation$new(type="calculation", function_exp="purrr::accumulate(.f = ~pmin(pmax(..1 + ..2, 0), 100), .x = tail(rain_max - as.numeric(evapotranspiration), n = -1), .init = 0)", result_name="wb_max", sub_calculations=list(rain_max))

The corrections involve two changes:

  1. Switching the function Change from using purrr::accumulate2 to purrr::accumulate
  2. Removing the .y parameter We no longer need the .y parameter since this is only needed in accumulate2.

Note this is just for when we have an evaporation variable given, but the reducing checkbox is not checked.

These changes should be applied to both the wb_max and wb_min calculations.