Fix sporadic KBinsDiscretizer mismatches with scikit-learn - #8467
Open
viclafargue wants to merge 1 commit into
Open
Fix sporadic KBinsDiscretizer mismatches with scikit-learn#8467viclafargue wants to merge 1 commit into
KBinsDiscretizer mismatches with scikit-learn#8467viclafargue wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3481 and #2933.
For the uniform strategy, the
KBinsDiscretizerestimator could occasionally assign values to different bins than scikit-learn when they were close to a bin edge. Two differences caused this:cupy.linspace. For float32 data, this could produce slightly different bin edges than passing a regular Python integer.This PR removes the tolerance and uses the same
searchsortedbehavior as the most recent version of scikit-learn. It also passes a Python integer tocupy.linspace, making uniform bin-edge generation consistent.For the quantile strategy, the test was originally marked as an expected failure because of bugs in older NumPy and CuPy percentile implementations. Those bugs have since been fixed.
Removing the quantile xfail works with scikit-learn 1.8 and earlier because its default linear interpolation matches cuML. Starting with scikit-learn 1.9, however, the default changed to
averaged_inverted_cdf. A direct comparison would therefore test two different quantile methods. This PR explicitly configures scikit-learn to use linear interpolation, ensuring that the test compares equivalent behavior while remaining compatible with older supported versions.