Skip to content

bmtr with efficient continous detection rates - #249

Merged
ethanplunkett merged 16 commits into
mainfrom
bmtr-method-api
Jul 28, 2026
Merged

ethanplunkett merged 16 commits into
mainfrom
bmtr-method-api

Conversation

@ethanplunkett

Copy link
Copy Markdown
Contributor

No description provided.

atharvashahane28 and others added 16 commits July 27, 2026 13:44
Replaces the `weighted`/`euclidean` boolean pair on `calc_bmtr()` with a
single `method` argument taking "binary" (default, `is_between()`),
"continuous" (fast planar geometry, recommended), or
"continuous-spherical" (slow great-circle, kept for comparison). Adds
`...`, forwarded to `calc_dist_weights()` for the continuous methods, so
the spread-kernel hyperparameters are tunable instead of hardcoded.

Splits the old dispatcher into one function per file:
`calc_spherical_detection_rate()` and `calc_euclidean_detection_rate()`
(plus its `project_points_simple()` helper) replace `weight_between()`
and `calc_detection_rate()`. `calc_dist_weights()`'s `method` argument is
renamed `kernel` to avoid colliding with the new `calc_bmtr()` `method`
once `...` forwards through both layers.

Bug fixes:
* `calc_bmtr()` passed `points`/`radius`/`n_directions` positionally into
  `weight_between(bf, weight_fun, points, radius, ...)`, so a
  user-supplied `points` landed in the unused `weight_fun` slot and was
  discarded whenever `weighted = TRUE`.
* Both continuous paths overwrote a user-supplied `radius` with
  `mean(res(bf))`. `radius` is now resolved and bounds-checked the same
  way `is_between()` does, with a separate `hull_buffer` for convex-hull
  sizing.
* `calc_dist_weights()` did `get(kernel)` rather than
  `get(paste0("k_", kernel))`, so the "m1"/"m5"/"sq" kernels errored with
  `object 'm1' not found` and were unusable.
* `calc_dist_weights_sd()` converted `dist_along_line`/`line_lengths`/
  `res_m` to km while leaving `gamma`/`kl` in meters, making the
  Matern-family kernels' contribution to the spread negligible next to
  the nugget at any realistic line length. Everything is now meters
  throughout.

Adds `visualize_distance_weights()` (exported) to plot the spread kernel
as an SD envelope or weight raster, so hyperparameters can be explored
before running the expensive `calc_bmtr()`. The `sd` computation is
factored into `calc_dist_weights_sd()` so both callers share it.

Retunes the `gamma`/`kl`/`s1` defaults from 40000/2000/10 — picked
against ~1-2 km demo lines — to 3e10/9e5/200, chosen by eye against the
envelope plots for the 100-150 km cell models BirdFlow actually uses.
The three have different units (variance, length, sqrt-length), so no
single rescaling factor applies.

The units fix and retuned defaults both change numeric output for the
continuous methods; snapshots updated accordingly.

Also adds a shared `small_test_bf()` fixture, test files for the two
detection-rate functions, `calc_dist_weights()`, and
`visualize_distance_weights()`, and fixes a stale
`calc_variance()` -> `calc_martern_variance()` call in
data-raw/martern_parameters.R.
Promotes the kernel hyperparameter tuning notebook from the local-only
dev/ directory to a tracked record at
data-raw/kernel_parameter_tuning.Rmd, alongside martern_parameters.R,
which plays the same role for the earlier defaults. data-raw/ is
.Rbuildignore'd, so this documents how the gamma/kl/s1 defaults were
chosen without shipping in the package or being built by R CMD check.

Tidied while moving:
* Dropped the units-bug history and dimensional-analysis sections, along
  with references to dev/kernel_units_issue.Rmd, which would have been
  dangling pointers into an untracked directory.
* Dropped the test-fixture section: it cited small_test_bf(), a
  tests/testthat/ helper unavailable outside the test suite, and compared
  parameter regimes that no longer exist anywhere in the repo.
* Added weight raster visualizations (type = "raster") for the final "m3"
  and "bb" parameters, with radius_m passed explicitly. The envelope plots
  show only the kernel's spread; the rasters show the weight
  calc_dist_weights() actually returns, which also depends on the
  detection radius.
* Clarified that gamma/kl/s1 are not resolution dependent. They model
  uncertainty in a biological process -- how far a bird's path strays from
  the straight line between two cells -- which is a property of the bird
  rather than of the grid. Resolution enters the kernel only through the
  nugget (res_m / 4), within-cell positional uncertainty, which scales
  itself. res_m = 100000 is used in the document purely as a reference
  scale for reading the plots. Spread magnitudes are now quoted in
  absolute distance rather than cell widths, which carried the same
  conflation.
* Corrected the gamma sweep prose, which described a range (2.5e10 to
  4.5e10 in 5e9 steps) that no longer matched the code
  (seq(1e10, 5e10, by = 1e10)), and referred to facets by cell count while
  the labels are in km.
* Pinned the date so re-knitting does not churn the diff.

Also repoints the NEWS.md reference at the notebook's new location, and
adds data-raw/*.html and data-raw/*_files/ to .gitignore so only the
source is tracked and the rendered output is regenerated by knitting.
Fixes problems in the Limitations section and `radius` documentation
describing the new `method` API:

* `method = "euclidean"` was not a valid value -- the choices are
  "binary", "continuous", and "continuous-spherical", so match.arg()
  would have rejected it. The great-circle straight-line assumption it
  was attached to belongs to "binary".
* An unbalanced backtick around `method = "continuous"` opened a markdown
  code span that never closed, emitting literal backticks into the
  generated Rd rather than \code{}.
* The Gaussian spread assumption was attributed to "continuous" alone,
  but "continuous-spherical" uses the same spread kernel; the two differ
  in geometry, not in whether a distribution is applied. Relatedly, the
  Gaussian was described as spreading around the great circle, but
  "continuous" is the one method that spreads around a straight line in
  the projected CRS. The great-circle/projected-CRS split is now stated
  separately from the Gaussian assumption.
* Reworded for parallel structure in the either/or construction, and
  rewrote the `radius` description of how the probability is integrated
  over the detection band.

Also corrects the continuous-detection example, which passed
`gamma = 60000` under a comment describing it as a wider spread kernel.
Since `gamma` now defaults to 3e10, that value was several orders of
magnitude narrower; it is now 6e10 with the default noted inline. The
example is shared into animate_bmtr() and plot_bmtr(), so their help
pages are regenerated too.
* kernel_parameter_tuning.Rmd: note that `gamma` and `kl` were tuned only
  for the default "m3" kernel, so the other Matern-family kernels will
  likely need their own values. Shorten the envelope plot's longest line
  from 50 to 30 cells.
* martern_parameters.R: add a header pointing at
  kernel_parameter_tuning.Rmd as the current record, and warning that this
  older script may not run correctly and that the km/m interpretation of
  some parameters has since changed.
@ethanplunkett
ethanplunkett merged commit d6fa8ea into main Jul 28, 2026
8 checks passed
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