Skip to content

[enhancement] unify policy creation in pybind11 #2390

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

Merged
merged 4 commits into from
May 31, 2025

Conversation

icfaust
Copy link
Contributor

@icfaust icfaust commented Mar 28, 2025

Description

Information about how to construct policies can be made at compile time rather than runtime checks. A common get_policy function is defined in pybind11 which is macro'ed or the various builds. It will return a viable oneDAL policy based on the python object given to it (it should be a queue). This will reduce the number of cycles and simplify the interface.

To enable this, the spmd_policy.cpp file is removed, and its contents are moved into the policy.cpp file (not sure why it was separated out anyway, especially since it was in a macro if statement).

Logic is slightly changed to throw an error if a queue with value other than None passed to a NO_DPC build, as this is unexpected behavior.


PR should start as a draft, then move to ready for review state after CI is passed and all applicable checkboxes are closed.
This approach ensures that reviewers don't spend extra time asking for regular requirements.

You can remove a checkbox as not applicable only if it doesn't relate to this PR in any way.
For example, PR with docs update doesn't require checkboxes for performance while PR with any change in actual code should have checkboxes and justify how this code change is expected to affect performance (or justification should be self-evident).

Checklist to comply with before moving PR from draft:

PR completeness and readability

  • 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 or created a separate PR with update 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 added a respective label(s) to PR if I have a permission for that.
  • 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.
  • I have extended testing suite if new functionality was introduced in this PR.

Performance

  • I have measured performance for affected algorithms using scikit-learn_bench and provided at least summary table with measured data, if performance change is expected.
  • I have provided justification why performance has changed or why changes are not expected.
  • I have provided justification why quality metrics have changed or why changes are not expected.
  • I have extended benchmarking suite and provided corresponding scikit-learn_bench PR if new measurable functionality was introduced in this PR.

@icfaust icfaust changed the title [enhanement] unify policy creation in pybind11 [enhancement] unify policy creation in pybind11 Mar 28, 2025
Copy link

codecov bot commented Mar 28, 2025

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
onedal/common/policy.cpp 33.33% 0 Missing and 4 partials ⚠️
Flag Coverage Δ
azure 79.91% <100.00%> (+0.04%) ⬆️
github 71.62% <42.85%> (ø)

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

Files with missing lines Coverage Δ
onedal/common/_backend.py 83.56% <100.00%> (+3.81%) ⬆️
onedal/dal.cpp 95.65% <ø> (ø)
onedal/common/policy.cpp 44.23% <33.33%> (-1.43%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@icfaust
Copy link
Contributor Author

icfaust commented Mar 28, 2025

/intelci: run

Copy link
Contributor

@ahuber21 ahuber21 left a comment

Choose a reason for hiding this comment

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

Let's try to get this merged. Could you quickly check the CI failures and see if they're fixed by simply re-running?

@ahuber21 ahuber21 added the enhancement New feature or request label Apr 23, 2025
@icfaust icfaust marked this pull request as ready for review May 31, 2025 16:57
@icfaust icfaust merged commit cf2dbcb into uxlfoundation:main May 31, 2025
28 of 29 checks passed
david-cortes-intel pushed a commit to david-cortes-intel/scikit-learn-intelex that referenced this pull request Jun 4, 2025
* create get_policy

* formatting

* Update policy.cpp
david-cortes-intel added a commit that referenced this pull request Jun 5, 2025
* add instructions for building with ASAN

* remove unneeded variable

* Update INSTALL.md

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* ENH: Enable model conversion from TreeLite (#2452)

* add conversion from treelite

* add dependency also for conda

* [enhancement, CI] add numpydoc validation to pre-commit (#2455)

* Update .pre-commit-config.yaml

* Update .pre-commit-config.yaml

* Update pyproject.toml

* Update pyproject.toml

* Update pyproject.toml

* Update .pre-commit-config.yaml

* Update .pre-commit-config.yaml

* Update pyproject.toml

* Update pyproject.toml

* Update dispatcher.py

* Update dispatcher.py

* Update .pre-commit-config.yaml

* Update pyproject.toml

* Update pyproject.toml

* Update incremental_basic_statistics.py

* Update incremental_linear.py

* Update incremental_linear.py

* Update incremental_linear.py

* Update _device_offload.py

* Update incremental_basic_statistics.py

* Update incremental_basic_statistics.py

* Update basic_statistics.py

* Update incremental_pca.py

* Update incremental_linear_model.py

* Update incremental_covariance.py

* Update incremental_ridge.py

* Update incremental_linear.py

* Update incremental_ridge.py

* make fixes

* Update _config.py

* Update _config.py

* Update _config.py

* Update _backend.py

* Update _dpep_helpers.py

* Update _dpep_helpers.py

* Update kernel_functions.py

* Update kernel_functions.py

* Update kernel_functions.py

* Update kernel_functions.py

* Update kernel_functions.py

* Update kernel_functions.py

* Update svm.py

* Update _data_conversion.py

* Update _data_conversion.py

* Update _data_conversion.py

* Update __init__.py

* Update __init__.py

* Update __init__.py

* Update _device_offload.py

* Update _device_offload.py

* Update covariance.py

* Update incremental_covariance.py

* Update incremental_covariance.py

* Update incremental_linear_model.py

* Update incremental_linear_model.py

* Update incremental_linear_model.py

* Update linear_model.py

* Update linear_model.py

* Update linear_model.py

* Update linear_model.py

* Update incremental_basic_statistics.py

* Update kmeans.py

* Update pca.py

* Update basic_statistics.py

* Update basic_statistics.py

* Update basic_statistics.py

* Update basic_statistics.py

* Update incremental_pca.py

* Update basic_statistics.py

* Update incremental_pca.py

* Update logistic_regression.py

* Update incremental_pca.py

* Update _config.py

* Update _config.py

* Update _device_offload.py

* Update _device_offload.py

* Update _device_offload.py

* Update _device_offload.py

* Update validation.py

* Update _sycl_queue_manager.py

* Update _sycl_queue_manager.py

* Update _sycl_queue_manager.py

* Update _sycl_queue_manager.py

* Update _backend.py

* Update _utils.py

* Update _utils.py

* Update _utils.py

* Update _utils.py

* Update _utils.py

* Apply suggestions from code review

Co-authored-by: david-cortes-intel <david.cortes@intel.com>

* Update _utils.py

* Update _device_offload.py

* Apply suggestions from code review

Co-authored-by: david-cortes-intel <david.cortes@intel.com>

---------

Co-authored-by: david-cortes-intel <david.cortes@intel.com>

* [enhancement] add ```__dlpack__``` and ```__dlpack_device__``` attributes to python oneDAL table class (#2430)

* initial save

* save point

* interim stop

* check:

* fix most problems

* save point

* formatting

* clang-formatting and save point

* add testing

* address coverage for dlpack::free_capsule

* formatting

* add gc and static_cast

* remove comment

* add suggestion

* chore(deps): update dependency xgboost to v3.0.1 (#2474)

* chore(deps): update dependency xgboost to v3.0.1

* Update requirements-test.txt

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* chore(deps): update dependency cython to v3.1.0 (#2470)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency cmake to v4.0.2 (#2468)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* DOC: Clarify that SPMD is linux-only (#2476)

* clarify that SPMD is not available on windows builds

* Update doc/sources/distributed-mode.rst

Co-authored-by: yuejiaointel <108152493+yuejiaointel@users.noreply.github.com>

---------

Co-authored-by: yuejiaointel <108152493+yuejiaointel@users.noreply.github.com>

* DOC: More details for conda installs (#2475)

* add conda-forge channel for dependencies, mention incompatibilities with anaconda

* update commands also in .md file

* suggest impi install from conda-forge

* remove warning about conda-forge incompatibilities

* Update INSTALL.md

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* Update doc/sources/distributed-mode.rst

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* Update doc/sources/distributed-mode.rst

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

---------

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* chore(deps): update dependency tornado to v6.5 [security] (#2478)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Migrate to modern NumPy interface (#2479)

* Migrate to modern NumPy interface

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>

* Update onedal/utils/validation.py

Co-authored-by: david-cortes-intel <david.cortes@intel.com>

* Update sklearnex/svm/_common.py

Co-authored-by: david-cortes-intel <david.cortes@intel.com>

---------

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Co-authored-by: david-cortes-intel <david.cortes@intel.com>

* fix: add cstdint include to transceiver for public CI (#2484)

* chore(deps): update dependency cython to v3.1.1 (#2483)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency numpy to v2.2.6 (#2481)

* chore(deps): update dependency numpy to v2.2.6

* Update dependencies-dev

* Update dependencies-dev

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* chore(deps): update dependency array-api-compat to v1.12.0 (#2477)

* chore(deps): update dependency array-api-compat to v1.12.0

* Update requirements-test.txt

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* downgrade setuptools to v79 for develop mode setup (#2480)

* chore(deps): update dependency xgboost to v3.0.2 (#2490)

* chore(deps): update dependency xgboost to v3.0.2

* Update requirements-test.txt

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* fix: use upload github action to public site (#2459)

* fix: use upload github action to public site

* fix: allow tag trigger with also year.month

* test: test tag trigger

* test: test tag trigger

* .github/workflows/docs-release.yml

* fix: don't copy gh-pages everytime, only copy parts don't exist

* fix:move prepare script to doc_release.sh

* fix:test checkout

* fix:test upload pages artifact only has v3

* fix:  fetch gh-apges if we have one

* fix: use eaceiris/actions-gh-pages@v3 to keep old files when upload new ones

* fix: use a intermediate unprotected branch for all doc archives

* fix: fix archive

* fix: fix archive

* fix: remove doc from switcher url in conf and archive enture folder

* fix: remove +x changes in doc relase before archive step

* fix: move permission and use exact commit id for github actions

* fix: use env var for _site folder and use doc folders for archive branch

* fix: delete everything for new doc arvhice branch

* fix: remove untracked files

* fix: only add verison folders to git first time doc archive

* fix: move TEMP_DOC_FOLDER to yml and make sure it is im tmp

* fix: move env to jobs

* fix: still need git rm for firs ttime doc archive creation

* fix: remove doc

* fix: allow manual trigger

* fix: workflow dispatch add input requriement

* fix: don't force input on workflow dispatch

* test: test older python version doc build warning

* fix: force doc version input

* fix: remove unused dependency

---------

Co-authored-by: yuejiaointel <41898282+github-actions[bot]@users.noreply.github.com>

* ci(workflows): add `OpenSSF Scorecard` GitHub Action (#2499)

* [CI, bugfix] correct stability issues in CondaRecipes (#2493)

* Update conda-recipe-lnx.yml

* Update conda-recipe-win.yml

* Update conda-recipe-lnx.yml

* Update conda-recipe-win.yml

* Update test_model_builders.py

* Update test_model_builders.py

* Delete tests/test_model_builders.py

* Revert "Delete tests/test_model_builders.py"

This reverts commit 453dd88.

* disable catboost and check

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* Update test_model_builders.py

* [CI] temporarily deselect ```test_dbscan_params_validation``` for green CI (#2500)

* Update deselected_tests.yaml

* Update deselected_tests.yaml

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

---------

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* [enhancement] unify policy creation in pybind11 (#2390)

* create get_policy

* formatting

* Update policy.cpp

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

* Update INSTALL.md

Co-authored-by: Ian Faust <icfaust@gmail.com>

---------

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>
Co-authored-by: Ian Faust <icfaust@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: yuejiaointel <108152493+yuejiaointel@users.noreply.github.com>
Co-authored-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Co-authored-by: yuejiaointel <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Aleksei Khomenko <aleksei.khomenko@intel.com>
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