Skip to content

Return 'cpu' for device(numpy_array) when dispatch is enabled#32705

Merged
ogrisel merged 3 commits intoscikit-learn:mainfrom
ogrisel:cpu-device-for-numpy
Nov 14, 2025
Merged

Return 'cpu' for device(numpy_array) when dispatch is enabled#32705
ogrisel merged 3 commits intoscikit-learn:mainfrom
ogrisel:cpu-device-for-numpy

Conversation

@ogrisel
Copy link
Copy Markdown
Member

@ogrisel ogrisel commented Nov 13, 2025

This PR will check using the CI whether we can change the current behavior for device to return "cpu" on numpy inputs when array_api_dispatch is True.

Context: #31829 (comment)

@github-actions github-actions bot removed the CUDA CI label Nov 13, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 13, 2025 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 9f3c267. Link to the linter CI: here

@ogrisel ogrisel mentioned this pull request Nov 13, 2025
Merged
@ogrisel ogrisel marked this pull request as ready for review November 13, 2025 22:22
@ogrisel ogrisel added Quick Review For PRs that are quick to review Array API labels Nov 13, 2025
@ogrisel ogrisel changed the title Return 'cpu' for device(numpy_array) when dispatch is enabled Return 'cpu' for device(numpy_array) when dispatch is enabled Nov 13, 2025
lesteve
lesteve reviewed Nov 14, 2025
View reviewed changes
@github-actions github-actions bot removed the CUDA CI label Nov 14, 2025
betatim
betatim approved these changes Nov 14, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@betatim betatim left a comment

Choose a reason for hiding this comment

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

I vote for this change. No opinion about including it in #31829 or merging this PR separately. Maybe separately makes it easier to understand why we made this change (for people from the future)?

@lucyleeow any opinions on this and where to include this change?

@lesteve
Copy link
Copy Markdown
Member

lesteve commented Nov 14, 2025

I pushed a small tweak in 9f3c267 (#32705). I slightly favour merging this PR separately from #31829.

@ogrisel
Copy link
Copy Markdown
Member Author

ogrisel commented Nov 14, 2025

LGTM as well. Let's merge.

@ogrisel ogrisel merged commit b4238b2 into scikit-learn:main Nov 14, 2025
38 checks passed
@ogrisel ogrisel deleted the cpu-device-for-numpy branch November 14, 2025 17:04
@virchan virchan mentioned this pull request Nov 14, 2025
Merged
@lucyleeow
Copy link
Copy Markdown
Member

lucyleeow commented Nov 15, 2025 •
edited
Loading

Sorry I am late to the party, I did some (probably unnecessary digging into numpy) just to understand support for None vs 'cpu' and found this: https://github.com/numpy/numpy/pull/25233/files#r1453641211

tl;dr it seems the spec defines the default value for device to be None, which is what numpy have done, even though 'cpu' is the only supported device. (edit: so for any to numpy conversions, both would be fine)

Thanks for this! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lesteve lesteve lesteve left review comments

@betatim betatim betatim approved these changes

Assignees

No one assigned

Labels

Array API No Changelog Needed Quick Review For PRs that are quick to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ogrisel @lesteve @lucyleeow @betatim