-
-
Notifications
You must be signed in to change notification settings - Fork 26.2k
FIX don't modify X in-place when precomputed in DBSCAN #27651
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
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.
We will need an entry in the changelog and mention it as a fix as well.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
# the sparse matrix. If `X` is modified in-place, the zeros from | ||
# the diagonal will be made explicit. | ||
np.fill_diagonal(X, 0) | ||
X = csr_container(X) |
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.
We could make an assert just below this line to be sure that we have implicit zeros.
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.
Otherwise LGTM.
Thanks @tataganesh
…rn into dbscan_matrix_copy
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.
Still +1 on my side.
We will need an additional reviewer.
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. Thanks @tataganesh
@tataganesh Could you kindly resolve the conflict? |
Reference Issues/PRs
See also #27508
What does this implement/fix? Explain your changes.
DBSCAN modifies the precomputed sparse matrix passed to the fit method, which can lead to undesirable behaviour. This PR creates a copy of the sparse matrix before it is modified.