Skip to content

[WIP, do NOT merge yet] ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU + document array API behavior#2677

Closed
david-cortes-intel wants to merge 4 commits intouxlfoundation:mainfrom
david-cortes-intel:fallback_to_sklearnex_cpu
Closed

[WIP, do NOT merge yet] ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU + document array API behavior#2677
david-cortes-intel wants to merge 4 commits intouxlfoundation:mainfrom
david-cortes-intel:fallback_to_sklearnex_cpu

Conversation

@david-cortes-intel
Copy link
Copy Markdown
Contributor

Description

Currently, when an operation is attempted on GPU and the operation is not supported by oneDAL, sklearnex will fall back straight away to sklearn on CPU, even if the operation might be supported by oneDAL on CPU.

This PR modifies the logic such that it will first check if the operation on CPU would be supported by oneDAL and fall back to it instead of to sklearn.

Along the way, it updates the docs about array API to reflect the current situation in unambiguous terms, and removes lots of outdated things in that page that no longer apply.

A few notes:

  • I'm not sure if oneDAL-CPU should have higher or lower priority than sklearn-arrayAPI. Currently, this will pass non-USM array-API inputs to sklearn without attempting a fallback to oneDAL-CPU, which I think is the most reasonable route.
  • I am not entirely sure that the mechanism here won't break any kind of contrived setup that's currently working. Please give it a thorough review.

Checklist:

Completeness and readability

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with updates and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.

@david-cortes-intel david-cortes-intel force-pushed the fallback_to_sklearnex_cpu branch from b55b5d7 to 6912794 Compare August 29, 2025 13:16
@david-cortes-intel david-cortes-intel changed the title ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU + document array API behavior Aug 29, 2025
@david-cortes-intel david-cortes-intel marked this pull request as draft August 29, 2025 13:20
@david-cortes-intel
Copy link
Copy Markdown
Contributor Author

/intelci: run

@david-cortes-intel
Copy link
Copy Markdown
Contributor Author

/intelci: run

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 29, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
sklearnex/_device_offload.py 40.00% 2 Missing and 1 partial ⚠️
Flag Coverage Δ
azure 80.73% <75.00%> (?)
github 73.23% <75.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
onedal/utils/_sycl_queue_manager.py 82.50% <100.00%> (+1.67%) ⬆️
sklearnex/_device_offload.py 80.72% <40.00%> (-2.62%) ⬇️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@david-cortes-intel david-cortes-intel changed the title ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU + document array API behavior [WIP, do NOT merge yet] ENH: Try fallback to sklearnex-CPU from GPU calls before fallback to sklearn-CPU + document array API behavior Sep 1, 2025
@icfaust
Copy link
Copy Markdown
Contributor

icfaust commented Sep 8, 2025

@david-cortes-intel Thanks for the work! If you wouldn't mind, could you split the docs changes off into a separate PR to simplify review/ focus theme/ speed integration? I know its still in draft, but just looking at the timeline for next week.

@david-cortes-intel
Copy link
Copy Markdown
Contributor Author

@david-cortes-intel Thanks for the work! If you wouldn't mind, could you split the docs changes off into a separate PR to simplify review/ focus theme/ speed integration? I know its still in draft, but just looking at the timeline for next week.

Sure, but this is not an urgent thing and not a high priority for the next release. It'd make more sense to first address the issue that we have with allow_fallback_to_host sometimes not working, otherwise the docs would have to describe a situation that's not very logical.

@david-cortes-intel
Copy link
Copy Markdown
Contributor Author

Closing in favor of #2707 for the docs part, and leaving the change in config for the future after potential bugs are addressed.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants