Skip to content

Don't infer dtype from model in cuml.explainer#7358

Merged
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.12from
jcrist:explainer-no-dtype-infer
Oct 17, 2025
Merged

Don't infer dtype from model in cuml.explainer#7358
rapids-bot[bot] merged 1 commit intorapidsai:branch-25.12from
jcrist:explainer-no-dtype-infer

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Oct 17, 2025

Previously the classes in cuml.explainer would try to infer the dtype of the model function from func.__self__.dtype. This only worked for some models (only those that exposed an undocumented .dtype attribute), and led to inconsistent behavior between model functions. As we cleanup our model internals we're also removing the .dtype attribute from models, so this method no longer works.

Aiming for consistency, we change the default of dtype in cuml.explainer to float32 everywhere - if a user wants to use float64 they'll need to specify it manually.

@jcrist jcrist self-assigned this Oct 17, 2025
@jcrist jcrist added Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function labels Oct 17, 2025
@jcrist jcrist requested a review from a team as a code owner October 17, 2025 02:29
@jcrist jcrist added the breaking Breaking change label Oct 17, 2025
@jcrist jcrist requested a review from csadorf October 17, 2025 02:29
@jcrist jcrist mentioned this pull request Oct 17, 2025
Copy link
Copy Markdown
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

Looks good. I have one change request.

Comment thread python/cuml/cuml/explainer/kernel_shap.pyx Outdated
Comment thread python/cuml/cuml/explainer/permutation_shap.pyx Outdated
@jcrist jcrist force-pushed the explainer-no-dtype-infer branch from dcf505c to 3d6d9b3 Compare October 17, 2025 16:00
Previously the classes in `cuml.explainer` would try to infer the dtype
of the model function from `func.__self__.dtype`. This only worked for
some models (only those that exposed an undocumented `.dtype`
attribute), and led to inconsistent behavior between model functions. As
we cleanup our model internals we're also removing the `.dtype`
attribute from models, so this method no longer works.

Aiming for consistency, we change the default of `dtype` in
`cuml.explainer` to `float32` everywhere - if a user wants to use
`float64` they'll need to specify it manually.
@jcrist jcrist force-pushed the explainer-no-dtype-infer branch from 3d6d9b3 to 631a6f9 Compare October 17, 2025 16:08
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Oct 17, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 37b6cc4 into rapidsai:branch-25.12 Oct 17, 2025
101 checks passed
@jcrist jcrist deleted the explainer-no-dtype-infer branch October 17, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants