-
Notifications
You must be signed in to change notification settings - Fork 211
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 Init Sparsity Support #2815
KMeans Init Sparsity Support #2815
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @md-shafiul-alam for thework done!
I see that only one test suit here updated. It worth to increase or maybe updates tests for better coverage of new fixes and features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM, but we have to discuss optional results, due to looks like its not necessary with the latest comments, and it makes sense to remove/update it. Also, it would be nice if you add tests with direct comparison between results on the same data, but with different formats(dense , sparse) with some tolerance to track that it works fine
/azp run CI |
Azure Pipelines successfully started running 1 pipeline(s). |
/intelci: run |
/intelci: run |
2 similar comments
/intelci: run |
/intelci: run |
/azp run CI |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/intelci: run |
The failures are not related. |
Add sparsity support to KMeans Init and fix a few bugs in daal sparse kmeans++ init, onedal kmeans++ init, and kmeans infer. Specific changes planned or made in this PR.
I have verified that