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

Jump to bottom

ENH Support for sparse matrices added to sklearn.metrics.silhouette_samples #24677

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

ENH Support for sparse matrices added to sklearn.metrics.silhouette_samples #24677

wants to merge 16 commits into from
+47 −8

Conversation

awinml
Copy link
Contributor

@awinml awinml commented Oct 16, 2022

Reference Issues/PRs

Fixes #18524
Fixes #18723

What does this implement/fix? Explain your changes.

Support for sparse matrices added to sklearn.metrics.silhouette_samples
Suggestions from the original stalled PR were implemented.

The changes update the reduce function used for computing the intra-cluster and inter-cluster distances. The current version is failing at, a) the pre-computed check for sparse matrices while getting the diagonal elements b) when trying to index a sparse matrix to pass weights to np.bincount function

  • Multiple merge conflicts with main were resolved.
  • New test test_silhouette_sparse_implementation was added to check implementation of silhouette_samples with sparse matrices.
  • Check for non-zero diagonal entries in silhouette_samples was updated to work with sparse matrices.

Sahil Gupta and others added 15 commits Nov 1, 2020
Updated silhouette reduce function
6b39cc3
Fixed lint tests
56c7bce
Codecov coverage failing test
d4f17da
Fixed indentation to conform to PEP style
5083c85
Fixed coding style
deffdfb
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn …
bb2d464 
…into feature/18524

Merge changes from remote
Unit test for changes
76e7140
Avoid converting sprase matrix to dense matrix
4e18cbf
Fixed line spaces in test file:
476e331
Merge branch 'master' of https://github.com/scikit-learn/scikit-learn …
15e400f 
…into feature/18524
Unit test for sparse input implementation
d1706a5
@awinml
Resolved Merge Conflicts in metrics/cluster/_unsupervised.py
b9de46f
@awinml
Updated with suggested change
0b1bee5
@awinml
Updated check for non-zero diagonal entries in silhouette_samples
72cd89c
@awinml
Fixed naming for error message
9c42182
@awinml awinml changed the title [MRG] Support for sparse matrices added to sklearn.metrics.silhouette_samples ENH Support for sparse matrices added to sklearn.metrics.silhouette_samples Oct 19, 2022
@awinml
Copy link
Contributor Author

awinml commented Oct 19, 2022

@thomasjpfan Would you kindly review this? I have made the suggested changes. I have also added a test to check implementation of silhouette_samples with sparse matrices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module:metrics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

sklearn.metrics.silhouette_samples does not work with sparse matrices
1 participant
@awinml