rapidsai / cuml

cuML - RAPIDS Machine Learning Library
https://docs.rapids.ai/api/cuml/stable/
Apache License 2.0
4.24k stars 532 forks source link

[BUG] cuml.manifold.UMAP does not work well. #4519

Closed gdaisukesuzuki closed 2 years ago

gdaisukesuzuki commented 2 years ago

Describe the bug by "pytest cuml/python/cuml/test/test_umap.py", all steps ended in fail with the messages. (I found it on both branch-21.12 and branch-22.02).

>   ???
E   TypeError: find_ab_params() takes exactly 2 positional arguments (3 given)

cuml/manifold/umap.pyx:473: TypeError

Patch to fix bugs I think this behavior comes from bug in
cuml/python/cuml/manifold/umap.pyx and it can be solved by applying the following patch

*** cuml/python/cuml/manifold/umap.pyx-dist     2022-01-25 22:55:33.931640327 +0900
--- cuml/python/cuml/manifold/umap.pyx  2022-01-25 23:14:48.295407566 +0900
***************
*** 470,476 ****
          free(umap_params)

      @staticmethod
!     def find_ab_params(spread, min_dist):
          """ Function taken from UMAP-learn : https://github.com/lmcinnes/umap
          Fit a, b params for the differentiable curve used in lower
          dimensional fuzzy simplicial complex construction. We want the
--- 472,478 ----
          free(umap_params)

      @staticmethod
!     def find_ab_params(self, spread, min_dist):
          """ Function taken from UMAP-learn : https://github.com/lmcinnes/umap
          Fit a, b params for the differentiable curve used in lower
          dimensional fuzzy simplicial complex construction. We want the
dantegd commented 2 years ago

Thanks for the issue @gdaisukesuzuki ! Indeed that patch fixes things, it'll be fixed by a PR very soon

github-actions[bot] commented 2 years ago

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

beckernick commented 2 years ago

Is this pytest failure still presenting?

gdaisukesuzuki commented 2 years ago

@beckernick No improvement. I did the same test with tag=v22.04.00 and got the same message.

viclafargue commented 2 years ago

Just saw the issue. Unfortunately I could not reproduce the pytests failures, maybe the problem is solved?

github-actions[bot] commented 2 years ago

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

viclafargue commented 2 years ago

I think that this issue is solved. Please do not hesitate to re-open if necessary.