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

Fix non-deterministic search #11

Closed

Conversation

pavlin-policar
Copy link

Fixes #10.

Copy over the following functions from UMAP master (ab7f27d):
build_candidates, new_build_candidates and make_nn_descent.

Honestly, I didn't look at the code at all, nor did I delve into how the method works. But I took the code for the problematic functions from the UMAP repository, which appears to be a fair bit ahead of this repository, and put it here and, voilà, it works.

I did identify the problem being the parallel=True directive in the old build_candidates. It seem that it was found and fixed a while ago in UMAP.

Copy over the following functions from UMAP master (ab7f27d):
build_candidates, new_build_candidates and make_nn_descent.
@tomwhite
Copy link
Collaborator

tomwhite commented Apr 3, 2019

@pavlin-policar I've turned your code to reproduce this issue into a unit test in #14. It has been fixed by #12, so this PR may no longer be needed.

I've also started work on harmonizing the the common code between umap and pynndescent (see #13), with the aim of ultimately removing the duplication entirely by making umap depend on pynndescent.

@pavlin-policar
Copy link
Author

Awesome, thanks!

@lmcinnes
Copy link
Owner

lmcinnes commented Apr 5, 2019

Thanks @tomwhite!

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.

Non-deterministic results with random_state
3 participants