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

KMeans OOP #1770

Merged
merged 150 commits into from
Sep 6, 2024
Merged

Conversation

md-shafiul-alam
Copy link
Contributor

@md-shafiul-alam md-shafiul-alam commented Mar 22, 2024

This PR intends to take KMeans algorithm out of preview namespace. The changes proposed:

  • KMeans out of preview
  • add sparsity support for KMeans
  • reduce number of deselected tests for KMeans

Nuances: KMeans Init does not have GPU support for sparse data. Hence CPU implementation is used using Host policy.

Based on #1634.
Needs to merge uxlfoundation/oneDAL#2815 first.

Fixes # - issue number(s) if exists

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes, if necessary (updated in # - add PR number)
  • The unit tests pass successfully.
  • I have run it locally and tested the changes extensively.
  • I have resolved any merge conflicts that might occur with the base branch.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)
  • I have added a respective label(s) to PR if I have a permission for that.

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@icfaust
Copy link
Contributor

icfaust commented Mar 25, 2024

I know its a draft, but this is a note for my future review: please enable all k_means GPU tests (i.e. remove from deselected_tests) and see what passes. This will be necessary to pull this code out of preview.

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@md-shafiul-alam
Copy link
Contributor Author

/azp run CI

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@md-shafiul-alam
Copy link
Contributor Author

/azp run CI

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor

/intelci: run

onedal/cluster/kmeans.py Show resolved Hide resolved
onedal/cluster/kmeans.py Show resolved Hide resolved
@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@md-shafiul-alam
Copy link
Contributor Author

/intelci: run

@md-shafiul-alam
Copy link
Contributor Author

The private CI warnings are not related.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great work on tests!

Copy link
Contributor

@ethanglaser ethanglaser left a comment

Choose a reason for hiding this comment

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

Great work on this. CI is in good shape and test coverage is nice. Pending resolution of any unresolved comments and manager approval.

@md-shafiul-alam md-shafiul-alam merged commit fede266 into uxlfoundation:main Sep 6, 2024
23 of 24 checks passed
md-shafiul-alam added a commit that referenced this pull request Sep 6, 2024
This reverts commit fede266.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants