Repository navigation
BiocParallel for parallel loops: workers started once per call, one BLAS/OpenMP thread per worker - #18
Merged
Conversation
…worker; reproducible relabelKontextual Parallel loops in Kontextual(), relabelKontextual(), getDistances(), getAbundances() and calcStateChanges() go through .par_lapply()/.par_mapply() (the spicyR helper): forked workers start once per call after the task exists and receive only task numbers, tasks go out in about eight chunks per worker, socket workers on Windows, and OpenBLAS runs one thread per worker during the call (RhpcBLASctl, set in the main session). Kontextual() builds point patterns, close pairs and edge correction in one pass over the images and uses one param for both parallel steps. relabelKontextual() seeds its random number streams from the session, so set.seed() makes it reproducible for any number of cores. Other results are identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW
Restoring several BLAS threads after forking restarts every OpenBLAS thread, and the new threads spin briefly (about 14 CPU-seconds per call with 128 threads). BLAS threads are now changed only when there is more than one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW
Forked workers can hang when the main session has used several OpenMP threads before forking (seen with randomForestSRC in ClassifyR). OpenMP threads are set to one for the call, as BLAS threads are, and restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW
There was a problem hiding this comment.
🟡 Changes recommended
Already-running worker pools bypass worker-side BLAS/OpenMP limits, undermining the central resource-control guarantee.
2 open findings
What changed in this PR
Introduces shared BiocParallel helpers to improve parallel-loop performance and control worker thread usage.
Changes:
- Adds parallel apply helpers with RNG and thread management.
- Migrates spatial calculations and relabelling to the helpers.
- Adds dependency, release notes, and parallel-behavior tests.
| File | Description |
|---|---|
R/utilities.R |
Implements BiocParallel helpers and thread controls. |
R/Statial.R |
Migrates distance, abundance, and state calculations. |
R/relabelKonditional.R |
Adds reproducible parallel relabelling. |
R/Konditional.R |
Consolidates image preprocessing and parallel calculations. |
tests/testthat/test-utilities.R |
Tests results, RNG behavior, and thread restoration. |
NEWS.txt |
Documents parallelization changes. |
NAMESPACE |
Removes obsolete imports. |
DESCRIPTION |
Bumps version and adds RhpcBLASctl. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+70
to
+75
| if (BiocParallel::bpisup(BPPARAM) || !is(BPPARAM, "MulticoreParam")) { | ||
| # Workers that are already running, or are new R sessions, are sent the task with each chunk. | ||
| threadVars <- Sys.getenv(c("OPENBLAS_NUM_THREADS", "OMP_NUM_THREADS"), unset = NA) | ||
| Sys.setenv(OPENBLAS_NUM_THREADS = 1L, OMP_NUM_THREADS = 1L) | ||
| on.exit(.restore_env(threadVars), add = TRUE) | ||
| return(BiocParallel::bplapply(X, task, BPPARAM = BPPARAM)) |
| task numbers, and tasks are handed out in about eight chunks per worker. Socket workers are used on | ||
| Windows. BLAS and OpenMP run one thread in each worker during the call (RhpcBLASctl, now imported); | ||
| OpenBLAS otherwise starts one thread per core in every worker. Kontextual() builds each image's point pattern, | ||
| close pairs and edge correction in one pass over the images. Results are unchanged. |
…per socket worker too Workers of a param that was already running set one BLAS and OpenMP thread themselves before their first task (settings changed in this session can't reach them). The eight-chunks-per-worker default now also applies to socket workers. NEWS no longer says all results are unchanged: relabelKontextual()'s relabelled images differ. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW
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.
Statial's parallel loops (
Kontextual(),relabelKontextual(),getDistances(),getAbundances(),calcStateChanges()) use one started-once BiocParallel pool per call, with one BLAS and OpenMP thread per worker. Version 1.15.4.Why. Bioconductor asks for BiocParallel rather than the parallel package. Two problems also made parallel runs slow on our servers:
OPENBLAS_NUM_THREADSin.Renvirondoes not help, because OpenBLAS reads it before.Renvironis loaded.MulticoreParamforks a new process for every chunk of tasks, and each forked process copies much of the session's memory when R collects garbage.The helper (
.par_lapplyinR/utilities.R):coresas a number or a BiocParallel param object.blas_set_num_threads(1)inside a worker would rebuild all 128 threads first. Workers forked after the session has used several OpenMP threads can hang.Xin a package environment, callsbpstart()once, sends only task numbers in about 8 chunks per worker, then callsbpstop().OPENBLAS_NUM_THREADS=1andOMP_NUM_THREADS=1before they start.Known fixed cost.
MulticoreParam()andbpstart()together run 2–3 fullgc()calls (.snowCoresMax()callsshowConnections(), which callsgc()). That is about 0.3 s each in a light session and 0.9 s each with SpatialExperiment data loaded. The cost per call does not depend on the size of the job.Changes beyond the helper:
Kontextual()used to fork five times per call. It now makes one pass over the images (point pattern, close pairs and edge correction), plus one pass for the core statistic, with a single param shared by both.relabelKontextual()used to ignoreset.seed()(MulticoreParamwithout anRNGseed), and its results changed with the number of cores. It now draws itsRNGseedfrom the session, so relabelled images are reproducible and the same for any number of cores. Its randomised rows therefore differ from older runs. Its "original" rows are identical..generateBPParamused.Random.seed[1](the RNG kind code) as its seed. It is removed.Results. Old and new are
identical()at 1 and 8 cores (kerenSCE) forKontextual()(8000 rows; also withedgeCorrect = FALSEandinhom = TRUE),getDistances,getAbundances,calcStateChangesand the original rows ofrelabelKontextual. All tests pass.Timing (8 cores):
Kontextual()is about the same (33.3 s against 31.2 s).relabelKontextual(nSim = 20)takes 326 s old and 178 s new.RhpcBLASctl added to Imports; the BiocParallel
importFroms are replaced byBiocParallel::calls.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y1PuRkfDQ3ApzuNoe5ovxW