Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenMP proof of concept #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

OpenMP proof of concept #8

wants to merge 2 commits into from

Conversation

LTLA
Copy link
Owner

@LTLA LTLA commented Sep 28, 2019

Tagging @mtmorgan as per our previous discussions on Slack.

Typical usage:

library(BiocSingular)
library(Matrix)
mat <- rsparsematrix(2e4, 1e5, density=0.01)
BiocSingular:::set_omp_threads(2L)
system.time(X <- BiocSingular:::compute_scale(mat, NULL))

A couple of points:

  • Uses set_omp_threads() (stolen from ShortRead) to control the number of OpenMP threads globally. This would be an excellent candidate for inclusion as an option in a BiocParallelParam object somewhere, such that entry into a bp*apply loop also sets the OpenMP threads. This would centralize control of OpenMP parallelization into the BiocParallel framework, and free developers from having to define their own OpenMP-related arguments in every parallelized functions.
  • beachmat's getters are maintained in serial sections. This is because they may involve calls back out to the R API, and it would probably be dicey to assume that R is happy with multiple threads calling it at the same time. As a result, this code saturates quite quickly with increasing cores as the loops are bottlenecked at the data loading step. Oh well.

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.

1 participant