b-cubed-eu / gcube

Simulation framework for biodiversity data cubes
https://b-cubed-eu.github.io/gcube/
Other
0 stars 2 forks source link

add simulate_timeseries #25

Closed wbarhdad closed 7 months ago

wbarhdad commented 7 months ago

Custom user functions need three arguments: initial_abundance, n_time_points and pars. Pars is a numeric vector that has any potential parameters for a function. Not sure if this is an acceptable approach in R.

wlangera commented 7 months ago

Very nice. I think better practise would be to use ... argument? We should see how we can use this.

PietrH commented 7 months ago

I agree to use the ... approach: https://www.r-bloggers.com/2013/01/the-three-dots-construct-in-r/

wbarhdad commented 7 months ago

@wlangera Implemented ... approach

wlangera commented 7 months ago

we need to change the name of the argument temporal_autocorr

wlangera commented 7 months ago

I pulled the main but there are still a lot of failures. @Annegreet I think it is better if your work is in a separate branche because this PR is getting too big. The part of Wissam can already be implemented, but your function is still in development (as far as I can tell).

@wbarhdad and @Annegreet can you sit together and discuss how to move further? I will ask @PietrH about the check failures.

PietrH commented 7 months ago

You've got a failing test as well as several R CMD CHECK issues. I'm looking into it.

PietrH commented 7 months ago

3 Expectations were failing, 2 were just missing characters in the error message expectation. I fixed those. The 3rd one is more complicated:

── Error ('test-simulate_timeseries.R:30:3'): arguments are of the right class ──
Error in `lambdas[i] <- lambdas[i - 1] + step`: replacement has length zero
Backtrace:
    ▆
 1. ├─testthat::expect_error(...) at test-simulate_timeseries.R:30:3
 2. │ └─testthat:::expect_condition_matching(...)
 3. │   └─testthat:::quasi_capture(...)
 4. │     ├─testthat (local) .capture(...)
 5. │     │ └─base::withCallingHandlers(...)
 6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 7. └─simcuber::simulate_timeseries(n_time_points = 1, temporal_autocorr = simulate_random_walk)
 8.   └─simcuber (local) temporal_autocorr(initial_average_abundance, n_time_points, seed = seed) at simcuber/R/simulate_timeseries.R:196:7

Failing around :

https://github.com/b-cubed-eu/simcuber/blob/796d996c3d554dc4896fc41f9b64f745523b62e1/R/simulate_timeseries.R#L195-L198

I'm investigating.

On a related note, the example of simulate_timeseries() is quite complex, and causing issues. Should we move it to a vignette instead?

PietrH commented 7 months ago

Actual issue is in simulate_random_walk()


  for (i in 2:n_time_points) {
    step <- stats::rnorm(1, mean = 0, sd = sd_step)
    lambdas[i] <- lambdas[i - 1] + step
  }

@wlangera