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

Using preprocessed-only birdflow objects as the bf argument for BirdFlowR functions #147

Closed
slager opened this issue Dec 7, 2023 · 1 comment · Fixed by #150
Closed

Comments

@slager
Copy link
Contributor

slager commented Dec 7, 2023

Using preprocessed-only birdflow objects as the bf argument for BirdFlowR functions is currently possible, but isn't explicitly supported and does not always produce expected results. However, it is sometimes very useful to be able to do so. For example, when wanting to extract timestep or CRS info created during the preprocessing.

The overall user experience related to using preprocessed-only objects could be improved with some combination of documentation, tests, warnings, or errors.

The following example from lookup_timestep() will soon be fixed by broader changes being made by @ethanplunkett, but similar situations might arise with other functions.

# goal: use existing birdflow object to lookup timesteps

bf <- BirdFlowR::preprocess_species('paibun', res = 1000, hdf5 = FALSE, skip_quality_checks = TRUE)
timesteps <-
  structure(
    c(1325376000, 1293408000, 1283040000),
    class = c("POSIXct",
              "POSIXt"),
    tzone = "UTC"
  )
tail(bf$dates, 1) #53
# timestep #53 in preprocessed model creates non-ascending `breaks` vector, 
# which causes error in findIntervals
lookup_timestep(timesteps, bf) # error, see above note

x <- findInterval(py, vec = breaks, all.inside = TRUE)

bf2 <- BirdFlowModels::amewoo
tail(bf2$dates, 1) #52
lookup_timestep(timesteps, bf2) # no error
@ethanplunkett ethanplunkett linked a pull request Dec 13, 2023 that will close this issue
@ethanplunkett
Copy link
Contributor

I think just about everything we'd expect to work now works:

  • lookup_timestep() now works as expected with date input as of (Preprocess 2022 #148)
  • lookup_timetep_sequence() works identically to a standard model.
  • Any use of the model as a spatial reference works identical to a full model.

A few things are still different between a preprocessed model relative to a standard model:

  • Lacks marginals and transitions
  • Has great circle distances
  • First distribution and dynamic mask are duplicated
  • First $date row is duplicated as last with timestep 53, week 1, and same date as first row.
  • Metadata related to fitting and importing is missing.

The implications of those differences:

  • n_distr() returns 53.
  • n_timesteps() returns 52 (somewhat arbitrary)
  • n_transitions() returns 52
  • is_cyclical() returns TRUE (somewhat arbitrary)
  • lookup_timestep("all") will return 1:53 (somewhat arbitrary but see next item)
  • get_distr() and get_dynamic_mask() will return all 53 with default "all" (passed to lookup_timetep())

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 a pull request may close this issue.

2 participants