Enhance coverage features and fix CRAN compliance issues#107
Open
hambrecht wants to merge 9 commits into
Open
Conversation
Worker nodes spawned by makeCluster had no access to generate.transects because the dssd package was never loaded in their session. Added a clusterEvalQ call to load dssd on all workers before dispatching reps.
Adds Depends: R (>= 3.6.0) as required by CRAN policy. R 3.6.0 is chosen as the floor since it deprecated warning(immediate.) and changed stringsAsFactors defaults. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two generate functions called st_buffer() without the sf:: prefix, relying on implicit namespace side-effects from import(sf). Made explicit to satisfy stricter CRAN namespace checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…alls The immediate. parameter in warning() was deprecated in R 3.6.0 and is actively flagged by CRAN checks in R 4.5+. Warnings display immediately by default, so removing the argument preserves behavior. Affects 18 files with ~60 call sites. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CRAN policy prohibits using T and F as substitutes for TRUE and FALSE because they are ordinary variables that can be overwritten by users. Replaced all ~150 instances across 11 files using the = T / = F argument pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The worker function was using get(".dssd_worker_state", envir = .GlobalEnv)
to retrieve data via a clusterEvalQ side-effect, which violates CRAN
policy on global environment modification. Replaced with the standard
pattern: export worker.state directly, then define and export worker.fun
that references it by name (available in the worker's own environment
after clusterExport).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The parallel package ships with every R installation as a recommended
base package, so requireNamespace("parallel") always returns TRUE.
The defensive check was misleading and added dead code. Removed the
outer if/else wrapper, keeping only the meaningful core-count checks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR modernizes warning/argument usage across the package and extends coverage simulations with an optional parallel execution path.
Changes:
- Added
run.parallel/max.coresparameters torun.coverage()and refactored per-rep coverage work into a helper to support parallel execution. - Standardized logical constants (
TRUE/FALSE),na.rm = TRUE, and removedimmediate.from warnings across multiple functions. - Updated package/roxygen metadata and a few namespace qualifications (e.g.,
sf::st_buffer).
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| R/write.transects.R | Standardizes warning call style (drops immediate.). |
| R/run.coverage.R | Adds parallel option and refactors coverage repetition execution. |
| R/generate.systematic.points.R | Standardizes warning call style (drops immediate.). |
| R/generate.segmented.grid.R | Qualifies st_buffer with sf:: and standardizes warnings. |
| R/generate.random.points.R | Standardizes warning call style (drops immediate.). |
| R/generate.parallel.lines.R | Qualifies st_buffer with sf:: and standardizes warnings. |
| R/generate.eqspace.zigzags.R | Standardizes warning call style (drops immediate.). |
| R/dssd-package.R | Adjusts _PACKAGE sentinel placement for roxygen/package docs. |
| R/check.shape.R | Standardizes warning call style (drops immediate.). |
| R/check.point.design.R | Standardizes warning call style (drops immediate.). |
| R/check.line.design.R | Standardizes warning call style (drops immediate.). |
| R/check.design.R | Uses na.rm = TRUE and standardizes warning call style. |
| R/calculate.trackline.segl.R | Standardizes logical constants in commented plotting code. |
| R/Survey.Design.R | Standardizes cat(..., fill=TRUE) and na.rm = TRUE. |
| R/Region.R | Standardizes warning call style (drops immediate.). |
| R/Point.Transect.R | Standardizes warning call style and logical constants in cat. |
| R/Point.Transect.Design.R | Standardizes warning call style (drops immediate.). |
| R/Line.Transect.R | Standardizes warning call style and logical constants in cat. |
| R/Line.Transect.Design.R | Standardizes warning call style (drops immediate.). |
| R/Coverage.Grid.R | Standardizes warning call style. |
| R/Class.Constructors.R | Standardizes warning call style (drops immediate.). |
| DESCRIPTION | Adds an R dependency constraint and updates roxygen config metadata. |
Files not reviewed (2)
- man/dssd-package.Rd: Language not supported
- man/run.coverage.Rd: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+113
to
+122
| my.cluster <- parallel::makeCluster(n.cores) | ||
| on.exit(parallel::stopCluster(my.cluster), add = TRUE) | ||
| parallel::clusterEvalQ(my.cluster, library(dssd)) | ||
| worker.state <- list(design = design, | ||
| pts = pts, | ||
| grid.count = grid.count, | ||
| save.transects = save.transects) | ||
| parallel::clusterExport(my.cluster, | ||
| varlist = c("worker.state", "run.single.coverage.rep", "inout"), | ||
| envir = environment()) |
Comment on lines
+131
to
+133
| rep.results <- parallel::parLapplyLB(my.cluster, X = as.list(1:reps), fun = worker.fun) | ||
| parallel::stopCluster(my.cluster) | ||
| on.exit() |
Comment on lines
+113
to
+114
| my.cluster <- parallel::makeCluster(n.cores) | ||
| on.exit(parallel::stopCluster(my.cluster), add = TRUE) |
Comment on lines
+132
to
+133
| parallel::stopCluster(my.cluster) | ||
| on.exit() |
| total.hits <- rep(0, grid.count) | ||
| for(rep in 1:reps){ | ||
| #Generate transects | ||
| run.single.coverage.rep <- function(rep, design, pts, grid.count, save.transects){ |
| } | ||
| if(any(!is.na(object@seg.length[index.neg]))){ | ||
| warning("Non NA values have been provided for segment length in strata where a segmented grid design was NOT selected. These vaues will be ignored.", immediate. = TRUE, call. = FALSE) | ||
| warning("Non NA values have been provided for segment length in strata where a segmented grid design was NOT selected. These vaues will be ignored.", call. = FALSE) |
| line.length[i] <- NA | ||
| }else if(!is.na(line.length[i]) && !is.na(samplers[i])){ | ||
| warning("Both sampers and line.length have been supplied for stratum ",i,", samplers argument will be ignored.", immediate. = TRUE, call. = FALSE) | ||
| warning("Both sampers and line.length have been supplied for stratum ",i,", samplers argument will be ignored.", call. = FALSE) |
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.
What this PR does
run.coverageto improve performance.Housekeeping included
TRUE/FALSE.Why
Coverage simulations can be computationally expensive, and this adds a parallel path for faster execution while preserving serial behavior. The housekeeping changes reduce warning noise, improve readability, and support long-term stability.
Impact
Validation performed
Reviewer focus