Repository navigation
bmtr with efficient continous detection rates - #249
Merged
Merged
Conversation
…weight_between function
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.
This was referenced Jul 28, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.