ENH: fallback to stock `scikit-learn` after `onedal` fail #1907

samir-nasibli · 2024-06-29T01:22:16Z

Description

In several estimators there are second level fallback flow, that is not obvious and controlled by the user.
Proposed to add control on fallback computation to stock scikit-learn after onedal backend in case of runtime error on onedal backend computations. config_context extended for allow_sklearn_after_onedal param, that control this behavior.
By default it is allowed to fallback to stock scikit-learn after onedal backend in case of runtime error. In sklearnex testing and sklearn conformance testing this is blocked, just to check onedal backend.

Done

new sklearnex config paramter to control fallback to stock scikit-learn if onedal fails
added new test case to check config_contex proper work: test_config_context_works
refactor and fixed test_set_config_works test suit.
added comments and docstrings.

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 # - add PR number)
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.

icfaust · 2024-07-08T05:07:46Z

@samir-nasibli do you need clarification on the background of this oddity in the codebase before we close this PR? It comes from these steps in the DAAL codebase: https://github.com/oneapi-src/oneDAL/blob/main/cpp/daal/src/algorithms/service_kernel_math.h#L721

samir-nasibli · 2024-07-08T06:58:35Z

@samir-nasibli do you need clarification on the background of this oddity in the codebase before we close this PR? It comes from these steps in the DAAL codebase: https://github.com/oneapi-src/oneDAL/blob/main/cpp/daal/src/algorithms/service_kernel_math.h#L721

@icfaust thank you! I am definitely not going to close this PR, some case needed to be addressed.

icfaust · 2024-07-08T07:03:47Z

@samir-nasibli do you need clarification on the background of this oddity in the codebase before we close this PR? It comes from these steps in the DAAL codebase: https://github.com/oneapi-src/oneDAL/blob/main/cpp/daal/src/algorithms/service_kernel_math.h#L721

@icfaust thank you! I am definitely not going to close this PR, some case needed to be addressed.

I'll wait to review until you mark it ready, I'm curious to see if you plan to refactor the fallback mechanism or fix a bug in the PLU factorization on oneDAL-side first.

samir-nasibli · 2024-07-08T07:09:06Z

@samir-nasibli do you need clarification on the background of this oddity in the codebase before we close this PR? It comes from these steps in the DAAL codebase: https://github.com/oneapi-src/oneDAL/blob/main/cpp/daal/src/algorithms/service_kernel_math.h#L721

@icfaust thank you! I am definitely not going to close this PR, some case needed to be addressed.

I'll wait to review until you mark it ready, I'm curious to see if you plan to refactor the fallback mechanism or fix a bug in the PLU factorization on oneDAL-side first.

I'll wait to review until you mark it ready, I'm curious to see if you plan to refactor the fallback mechanism or fix a bug in the PLU factorization on oneDAL-side first.

Please read the short description to the PR more carefully.
At the moment there is no ready-made solution for a particular case, there is a draft PR, in the future it is planned to manage this through config_context.

When the proposal is expanded and ready for discussion/review, the PR will be marked ready for review
Thank you!

samir-nasibli · 2024-08-09T10:30:52Z

/intelci: run

samir-nasibli · 2024-08-09T20:59:21Z

Linear regression fails all tests on both GPU and CPU, that requires additional investigation.

I will separate deprecation and changes for the LogReg into the separate PR, since this eventually passed all tests. See #1996

samir-nasibli · 2024-09-04T21:28:38Z

The statistics of the onedal4py backend usage and sklearn after onedal4py backend should be investigate.
It seems LinReg has some defects, that should be addressed.

The mechanism itself for falling back sklearn_after_onedal will be updated via config_context.

Follow up tickets will be created.

samir-nasibli · 2024-09-05T19:33:11Z

/intelci: run

onedal/_config.py

…mir-nasibli/scikit-learn-intelex into dep/skl_fallback_after_onedal

samir-nasibli · 2024-10-19T22:12:04Z

/intelci: run

sklearnex/tests/test_config.py

sklearnex/_config.py

icfaust

This is impacted by #2083 , that would mean only LinearRegression GPU, and LogisticRegression GPU estimators are influenced. Is it worth having a new config keyword for these specific cases? @Alexsandruss should weigh in. I'm still in favor of closing this PR. I would say the removal can occur naturally when these issues are addressed in the individual estimators.

icfaust · 2024-10-21T11:29:04Z

Forgot to mention @avolkov-intel for comment on this as well.

samir-nasibli · 2024-10-21T12:16:55Z

This is impacted by #2083 , that would mean only LinearRegression GPU, and LogisticRegression GPU estimators are influenced. Is it worth having a new config keyword for these specific cases? @Alexsandruss should weigh in. I'm still in favor of closing this PR. I would say the removal can occur naturally when these issues are addressed in the individual estimators.

This PR doesn't change py default anything. Only on testing we are removing these.
We are already have flag for example allowing do computations on onedal cpu if gpu not supported, and it is disabled for testing. The same idea here. If this cases will be addressed then we can remove totally this flag from config_context.

.circleci/run_xpu_tests.py

sklearnex/tests/test_config.py

fixed set_config for the config default values

samir-nasibli · 2024-10-28T15:28:19Z

/intelci: run

Alexsandruss · 2024-10-29T12:07:36Z

sklearnex/tests/test_config.py

 def test_set_config_works():
+    """Test validates that the config settings were applied correctly by
+    set_config.
+    """


These tests will require generalization and parameterization in future.

Thank you @Alexsandruss . Please suggest inputs I will cover with a ticket

…ion#1907) * ENH: fallback to stock `scikit-learn` after `onedal` fail * fix `allow_sklearn_after_onedal` setting * covered config_context with more tests * fixed set_config for the config default values

DEP: deprecate fallback to sklearn after onedal

34346c8

Merge branch 'intel:main' into dep/skl_fallback_after_onedal

64874e7

samir-nasibli marked this pull request as ready for review August 9, 2024 20:54

samir-nasibli requested a review from Alexsandruss as a code owner August 9, 2024 20:54

samir-nasibli requested review from icfaust, ethanglaser, olegkkruglov, md-shafiul-alam and avolkov-intel August 9, 2024 20:54

samir-nasibli marked this pull request as draft August 9, 2024 20:56

samir-nasibli mentioned this pull request Sep 4, 2024

DEP: deprecate fallback to sklearn after onedal in LogReg #1996

Closed

10 tasks

samir-nasibli added 2 commits September 4, 2024 10:52

Merge branch 'intel:main' into dep/skl_fallback_after_onedal

1d79f01

revert changes for logistic_regression. Moved to uxlfoundation#1996

b042b32

samir-nasibli changed the title ~~DEP: deprecate fallback to sklearn after onedal~~ DEP: deprecate fallback to sklearn after onedal for LinRegression Sep 4, 2024

samir-nasibli added 2 commits September 5, 2024 10:57

Merge branch 'main' into dep/skl_fallback_after_onedal

5877ecb

adding config_context flag for the fallback

c627e8d

samir-nasibli changed the title ~~DEP: deprecate fallback to sklearn after onedal for LinRegression~~ DEP: fallback to sklearn after onedal for LinRegression Sep 27, 2024

samir-nasibli changed the title ~~DEP: fallback to sklearn after onedal for LinRegression~~ DEP: fallback to sklearn after onedal Sep 27, 2024

samir-nasibli changed the title ~~DEP: fallback to sklearn after onedal~~ DEP: fallback to sklearn after onedal fail Sep 27, 2024

samir-nasibli commented Sep 27, 2024

View reviewed changes

onedal/_config.py Show resolved Hide resolved

samir-nasibli changed the title ~~DEP: fallback to sklearn after onedal fail~~ ENH: fallback to sklearn after onedal fail Sep 27, 2024

Merge branch 'intel:main' into dep/skl_fallback_after_onedal

45f3831

samir-nasibli added 4 commits October 19, 2024 23:17

Merge branch 'intel:main' into dep/skl_fallback_after_onedal

12af875

updated for LogReg

f815a7e

Merge branch 'dep/skl_fallback_after_onedal' of https://github.com/sa…

2f21b85

…mir-nasibli/scikit-learn-intelex into dep/skl_fallback_after_onedal

disabled allow_sklearn_after_onedal for conformnance testing

206f4a3

samir-nasibli added the enhancement New feature or request label Oct 19, 2024

samir-nasibli commented Oct 19, 2024

View reviewed changes

sklearnex/tests/test_config.py Outdated Show resolved Hide resolved

samir-nasibli changed the title ~~ENH: fallback to sklearn after onedal fail~~ ENH: fallback to stock scikit-learn after onedal fail Oct 19, 2024

samir-nasibli commented Oct 19, 2024

View reviewed changes

sklearnex/_config.py Outdated Show resolved Hide resolved

samir-nasibli marked this pull request as ready for review October 19, 2024 23:02

samir-nasibli requested review from napetrov, homksei and ahuber21 as code owners October 19, 2024 23:02

icfaust reviewed Oct 21, 2024

View reviewed changes

samir-nasibli requested a review from icfaust October 21, 2024 13:05

Alexsandruss requested changes Oct 23, 2024

View reviewed changes

.circleci/run_xpu_tests.py Show resolved Hide resolved

fix allow_sklearn_after_onedal setting

50d569f

samir-nasibli commented Oct 23, 2024

View reviewed changes

sklearnex/tests/test_config.py Show resolved Hide resolved

covered config_context with more tests

205289e

fixed set_config for the config default values

samir-nasibli requested a review from Alexsandruss October 28, 2024 15:21

Merge branch 'intel:main' into dep/skl_fallback_after_onedal

a3e696b

Alexsandruss approved these changes Oct 29, 2024

View reviewed changes

samir-nasibli merged commit b61cc90 into uxlfoundation:main Oct 30, 2024
26 checks passed

samir-nasibli deleted the dep/skl_fallback_after_onedal branch October 30, 2024 08:01

samir-nasibli mentioned this pull request Nov 4, 2024

TEST: disabled GPU test case #2149

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: fallback to stock `scikit-learn` after `onedal` fail #1907

ENH: fallback to stock `scikit-learn` after `onedal` fail #1907

samir-nasibli commented Jun 29, 2024 •

edited

Loading

icfaust commented Jul 8, 2024

samir-nasibli commented Jul 8, 2024

icfaust commented Jul 8, 2024

samir-nasibli commented Jul 8, 2024

samir-nasibli commented Aug 9, 2024

samir-nasibli commented Aug 9, 2024 •

edited

Loading

samir-nasibli commented Sep 4, 2024

samir-nasibli commented Sep 5, 2024

samir-nasibli commented Oct 19, 2024

icfaust left a comment

icfaust commented Oct 21, 2024

samir-nasibli commented Oct 21, 2024 •

edited

Loading

samir-nasibli commented Oct 28, 2024

Alexsandruss Oct 29, 2024

samir-nasibli Oct 30, 2024

ENH: fallback to stock scikit-learn after onedal fail #1907

ENH: fallback to stock scikit-learn after onedal fail #1907

Conversation

samir-nasibli commented Jun 29, 2024 • edited Loading

Description

Done

icfaust commented Jul 8, 2024

samir-nasibli commented Jul 8, 2024

icfaust commented Jul 8, 2024

samir-nasibli commented Jul 8, 2024

samir-nasibli commented Aug 9, 2024

samir-nasibli commented Aug 9, 2024 • edited Loading

samir-nasibli commented Sep 4, 2024

samir-nasibli commented Sep 5, 2024

samir-nasibli commented Oct 19, 2024

icfaust left a comment

Choose a reason for hiding this comment

icfaust commented Oct 21, 2024

samir-nasibli commented Oct 21, 2024 • edited Loading

samir-nasibli commented Oct 28, 2024

Alexsandruss Oct 29, 2024

Choose a reason for hiding this comment

samir-nasibli Oct 30, 2024

Choose a reason for hiding this comment

ENH: fallback to stock `scikit-learn` after `onedal` fail #1907

ENH: fallback to stock `scikit-learn` after `onedal` fail #1907

samir-nasibli commented Jun 29, 2024 •

edited

Loading

samir-nasibli commented Aug 9, 2024 •

edited

Loading

samir-nasibli commented Oct 21, 2024 •

edited

Loading