Issue
Hi, I'm running reverse dependency checks on the future package, and I noticed that I caused the CPU load to explode on a 96-core machine that I shared with several other users.
I tracked it down to your ppcseq package that uses ncores = detect_cores() by default, where:
|
#' @importFrom parallel detectCores |
|
detect_cores = function(){ |
|
|
|
detectCores() %>% as.integer |
|
|
|
# if(.Platform$OS.type == "unix") |
|
# system("nproc", intern = TRUE) %>% as.integer %>% sum(-1) |
|
# else if(.Platform$OS.type == "windows") |
|
# parallel::detectCores() %>% as.integer %>% sum(-1) |
|
# else stop("Your platform type is not recognised") |
|
|
|
} |
Suggestion
FWIW, over on CRAN, they ask you to test with a maximum of two cores, e.g. max(2L, parallel::detectCores()). On Bioconductor, I think they target 4 cores, e.g. BiocParallel respects environment variables BBS_HOME and BIOCPARALLEL_WORKER_NUMBER, and the former is set to 4 on their test machines. So, you could make your detect_cores() agile to those. If so, then people like I, but also sysadms, could set those environment variables to make your package run nicer on machines where there are other users running at the same time.
(Disclaimer: I'm the author). Alternatively, you could just replace parallel::detectCores() with parallelly::availableCores(). That functions respects above environment variables and much more, including nproc (as you've prototyped but commented out), and HPC scheduler allocations. It's designed to make your package play nice by respecting various kind of system and env var settings, cf. https://parallelly.futureverse.org/#availablecores-vs-paralleldetectcores.
Thanks.
Issue
Hi, I'm running reverse dependency checks on the future package, and I noticed that I caused the CPU load to explode on a 96-core machine that I shared with several other users.
I tracked it down to your ppcseq package that uses
ncores = detect_cores()by default, where:ppcseq/R/utilities.R
Lines 931 to 942 in c430833
Suggestion
FWIW, over on CRAN, they ask you to test with a maximum of two cores, e.g.
max(2L, parallel::detectCores()). On Bioconductor, I think they target 4 cores, e.g. BiocParallel respects environment variablesBBS_HOMEandBIOCPARALLEL_WORKER_NUMBER, and the former is set to4on their test machines. So, you could make yourdetect_cores()agile to those. If so, then people like I, but also sysadms, could set those environment variables to make your package run nicer on machines where there are other users running at the same time.(Disclaimer: I'm the author). Alternatively, you could just replace
parallel::detectCores()withparallelly::availableCores(). That functions respects above environment variables and much more, includingnproc(as you've prototyped but commented out), and HPC scheduler allocations. It's designed to make your package play nice by respecting various kind of system and env var settings, cf. https://parallelly.futureverse.org/#availablecores-vs-paralleldetectcores.Thanks.