Skip to content
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

ENH: functional support for Array API #1861

Merged
merged 105 commits into from
Sep 10, 2024

Conversation

samir-nasibli
Copy link
Contributor

@samir-nasibli samir-nasibli commented Jun 12, 2024

Description

Array API functional support without zero-copy in scikit-learn-intelex.
Depends on #1999 and #1998

Brings extension for the device_offload logic and extends support_usm_ndarray decorator and make this work also for the array-api-strict inputs. New support_array_api decorator mainly impl Array API functional support in scikit-learn-intelex.

Proposed changes shown on the dataflow diagram below. Yellow plates is extensions in the sklearnex dataflow diagram and show the first iteration of the scikit-learn-intelex Array API support (functional support).

image

Works fine with array-api-strict v1 and v2 as well. Some test suits requires updates for the v2, that will be solved on #2012

Changes includes:

  • updates on _device_offload modules both of onedal4py and sklearnex.
    • extensions for sklearnex._device_offload.wrap_output_data decorator and sklearnex._device_offload.dispatch function.
    • onedal._device_offload._transfer_to_host extended for the Array API inputs.
    • onedal._device_offload.support_usm_ndarray renamed to onedal._device_offload.support_array_api extend for Array API inputs handling. (several files changed for this this renaming)
  • onedal4py's testing utility get_dataframes_and_queues extended for the array_api inputs
  • _array_api modules added in the both onedal4py and sklearnex.\
    • onedal.utils._array_api module includes several array api utilities and _get_sycl_namespace that is used in the get_namespace (from sklearnex.utils._arrray_api), that returns supported array api namespace.

Changes are documented in the files. General documentation for Array API support will be able at #1918

Test statistics

More than 2834 new test cases (+ 46%) added for Array API inputs. On sklearnex testing you may observe the rise of test pass statistics
from
5943 passed, 898 skipped, 42083 warnings in 394.72s (0:06:34)
to
8674 passed, 1001 skipped, 44350 warnings in 538.83s (0:08:58)

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes, if necessary (updated in DOC: Array API support #1918)
  • The unit tests pass successfully.
  • I have run it locally and tested the changes extensively.
  • I have resolved any merge conflicts that might occur with the base branch.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)
  • I have added a respective label(s) to PR if I have a permission for that.

@samir-nasibli samir-nasibli changed the title Enh/functional array api ENH: functional support for Array API Jun 12, 2024
@samir-nasibli
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor Author

/intelci: run

CI is Green. Test fails are related #2031

@samir-nasibli samir-nasibli marked this pull request as ready for review September 7, 2024 06:56
@samir-nasibli samir-nasibli mentioned this pull request Sep 7, 2024
10 tasks
Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

Left a comment elsewhere on a previous review item, just a quick check. Looking pretty good, will give another review by tomorrow.

onedal/_device_offload.py Outdated Show resolved Hide resolved
@samir-nasibli
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor Author

/intelci: run

Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

I have verified the extension of testing for sklearn > 1.2 for array_api, and looked at the performance numbers, small comments to answer before approval. Any update on who else has glanced at this, or is unwilling to review?

onedal/_device_offload.py Show resolved Hide resolved
onedal/_device_offload.py Show resolved Hide resolved
onedal/_device_offload.py Show resolved Hide resolved
onedal/utils/_array_api.py Show resolved Hide resolved
onedal/_device_offload.py Show resolved Hide resolved
@samir-nasibli
Copy link
Contributor Author

/intelci: run

Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

ready to go

onedal/_device_offload.py Show resolved Hide resolved
@samir-nasibli samir-nasibli merged commit c0181f5 into intel:main Sep 10, 2024
24 checks passed
@samir-nasibli samir-nasibli deleted the enh/functional_array_api branch September 10, 2024 15:32
samir-nasibli added a commit to samir-nasibli/scikit-learn-intelex that referenced this pull request Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Array API enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants