Skip to content

lets umap run in parallel #3295

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

lets umap run in parallel #3295

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fbc2e49
lets umap run in parallel
Intron7 Oct 18, 2024
9ce1770
Merge branch 'main' into umap-parallel
Intron7 Oct 18, 2024
812e630
add a test to check if parallel with randomstate errors
Intron7 Oct 18, 2024
ba6538d
updated docstring
Intron7 Oct 18, 2024
d2ab85d
added warning
Intron7 Oct 18, 2024
4eba4a8
update test
Intron7 Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/scanpy/tools/_umap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ def umap(
# for the init condition in the UMAP embedding
default_epochs = 500 if neighbors["connectivities"].shape[0] <= 10000 else 200
n_epochs = default_epochs if maxiter is None else maxiter
if parallel and random_state is not None:
warnings.warn(
"Parallel execution was expected to be disabled when both `parallel=True` and `random_state` are set, "
"to ensure reproducibility. However, parallel execution still seems to occur, which may lead to "
"non-deterministic results."
)
Comment on lines +219 to +224
Copy link
Contributor

@ilan-gold ilan-gold Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to the umap repo to give user more context. Something like "UMAP reports that parallel execution should error with a random seed to ensure reproducibility. Parallel execution may occur nonetheless, which can lead to non-deterministic results." with a link to their docs

Copy link
Contributor

@ilan-gold ilan-gold Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then make sure this warning is raised in tests

Copy link
Member Author

@Intron7 Intron7 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this once i have clarification from lmcinnes/umap#1155

X_umap, _ = simplicial_set_embedding(
data=X,
graph=neighbors["connectivities"].tocoo(),
Expand Down
Loading