FEA: Ridge support for Array API compliant inputs by fcharras · Pull Request #27800 · scikit-learn/scikit-learn

fcharras · 2023-11-17T13:51:26Z

What does this implement/fix? Explain your changes.

Follow-up to #26024

This PR is a WIP that aims at bringing support of Ridge for Array API compliant inputs, starting with SVD solver.

Co-authored-by: Tim Head <betatim@gmail.com>

Some Array API compatible libraries do not have a device called 'cpu'. Instead we try and detect the lib+device combination that does not support float64.

…27073)

…test_affinity_propagation` (scikit-learn#27095) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

…argument (scikit-learn#26814)

github-actions · 2023-11-17T13:52:49Z

✔️ Linting Passed

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

_{Generated for commit: a02e7a4. Link to the linter CI: here}

sklearn/utils/extmath.py

…nto ENH/r2_score_array_api

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lorentzenchr · 2024-03-26T17:02:19Z

Also I thought on the way that factoring the file _ridge.py a bit could be a good idea to have all config validation at the same place. At the moment there is some in Ridge.fit and some in _ridge_regression (and some checks are done twice) and there is a bizarre redundancy and inconsistency in use of some config keywords. I finally gave up because it started to look very complicated.

This this and this taken together is very confusing, but there is also this (other solvers just check if X_offset is None) and in general I don't understand if there's supposed to be a performance interest in using sag with return_intercept compared to other solvers with intercept computed separately...

I sympathize with you and tried to improve the code base at least a bit in the past. For the sake of this PR, I would advise to focus on the array API only.

sklearn/linear_model/tests/test_ridge.py

sklearn/linear_model/_ridge.py

ogrisel

LGTM once all occurrences of {xp.__name__} are actually interpolated to make error messages more informative.

fcharras · 2024-03-27T12:51:27Z

@lorentzenchr thank you for the sympathy 😁 I realize my message maybe sounded harsh, it was not meant to be, sorry about that.

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

… tests

betatim · 2024-04-05T12:23:11Z

sklearn/linear_model/_ridge.py

+
+    if positive:
+        raise ValueError(
+            "The solvers that support positive fitting do not support "


What is "positive fitting"? Is there a different word/words that explain it or am I a rare person for not knowing what it is? Basically, can we make the error message less jargon-y?

We should probably rephrase to "fitting with positivity constraints" or similar.

sklearn/linear_model/_ridge.py

Co-authored-by: Tim Head <betatim@gmail.com>

ogrisel · 2024-04-24T12:41:00Z

@betatim Franck told me he wouldn't have the time to address your feedback this week so I took the liberty to do it.

There is an unrelated CI failure at APT install time for the ubuntu_atlas job:

Reading package lists...
E: Failed to fetch https://packages.microsoft.com/ubuntu/22.04/prod/dists/jammy/InRelease  Clearsigned file isn't valid, got 'NOSPLIT' (does the network require authentication?)
E: The repository 'https://packages.microsoft.com/ubuntu/22.04/prod jammy InRelease' is no longer signed.

Hopefully it's transient (although it still fails after clicking the re-run failed jobs button).

EDIT: the last re-run on Azure did work.

betatim · 2024-04-25T09:52:00Z

Thanks for all the work Franck and then taking over Olivier. Looks good to me and the robots are happy, so I'll click merge.

elindgren and others added 19 commits August 18, 2023 14:16

update r2 score to use the array API, and write initial tests

e0429db

Merge remote-tracking branch 'upstream/main'

b9c1720

Merge branch 'main' into ENH/r2_score_array_api

5666ce5

Merge branch 'main' into ENH/r2_score_array_api

4580d1c

Fix some review comments and move stuff to CPU

a4dd594

Add regression tests to the test_common framework

adc7680

Update sklearn/metrics/tests/test_regression.py

85469a9

Co-authored-by: Tim Head <betatim@gmail.com>

Update sklearn/metrics/tests/test_regression.py

b7efaa5

Co-authored-by: Tim Head <betatim@gmail.com>

Remove hardcoded device choice in _weighted_sum

ac533c2

Some Array API compatible libraries do not have a device called 'cpu'. Instead we try and detect the lib+device combination that does not support float64.

Factor out max float precision determination

35be22e

Use convenience function to find highest accuracy float in r2_score

7c53e19

add tests for _average for Array API

230ae46

MNT Ignore ruff errors (scikit-learn#27094)

e4672d1

DOC fix docstring for sklearn.datasets.get_data_home (scikit-learn#…

8ba9485

…27073)

TST Extend tests for scipy.sparse.*array in `sklearn/cluster/tests/…

490e0b4

…test_affinity_propagation` (scikit-learn#27095) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

MNT Remove DeprecationWarning for scipy.sparse.linalg.cg tol vs rtol …

a8a820c

…argument (scikit-learn#26814)

Merge branch 'main' into ENH/r2_score_array_api

552e421

Merge remote-tracking branch 'upstream/main' into ENH/r2_score_array_api

ff52710

remove temporary file

fe9cc1c

ogrisel reviewed Nov 17, 2023

View reviewed changes

sklearn/utils/extmath.py Show resolved Hide resolved

fcharras force-pushed the enh/ridge_support_for_array_api branch 3 times, most recently from c84731a to 8c38446 Compare November 21, 2023 12:23

fcharras mentioned this pull request Nov 22, 2023

Use Array API in r2_score #27102

Closed

fcharras added 5 commits December 5, 2023 16:16

WIP: solving dtype and device maze

93257ba

Fix changelog conflict

45bbe4e

Tests fixups

2145a6b

Tests fixups

bd4b224

Merge branch 'main' of https://github.com/scikit-learn/scikit-learn i…

34aceb1

…nto ENH/r2_score_array_api

Adress review suggestions

9745941

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

ogrisel reviewed Mar 27, 2024

View reviewed changes

sklearn/linear_model/tests/test_ridge.py Outdated Show resolved Hide resolved

ogrisel reviewed Mar 27, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

ogrisel reviewed Mar 27, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

ogrisel reviewed Mar 27, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

ogrisel approved these changes Mar 27, 2024

View reviewed changes

fcharras and others added 3 commits March 27, 2024 18:29

Properly interpolate strings.

741a86c

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

Also fix typo and use helper and properly interpolate strings in unit…

4edebac

… tests

Update credits

5600a2a

ogrisel added the module:linear_model label Mar 28, 2024

Merge branch 'main' into enh/ridge_support_for_array_api

fc6521b

lithomas1 mentioned this pull request Apr 2, 2024

ENH: Add Dask Array API support #28588

Closed

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Show resolved Hide resolved

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

betatim reviewed Apr 5, 2024

View reviewed changes

sklearn/linear_model/_ridge.py Outdated Show resolved Hide resolved

ogrisel and others added 5 commits April 12, 2024 09:42

Merge branch 'main' into enh/ridge_support_for_array_api

25da280

Typo

1a29866

Co-authored-by: Tim Head <betatim@gmail.com>

Rename solver resolution helper functions

e51d6e4

Remove useless xp.asarray(intercept) in the saga branch

6a84850

Improved warning message based on Tim's suggestion

a02e7a4

betatim approved these changes Apr 25, 2024

View reviewed changes

betatim merged commit 171e124 into scikit-learn:main Apr 25, 2024

Uh oh!

Conversation

fcharras commented Nov 17, 2023 • edited by ogrisel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this implement/fix? Explain your changes.

Uh oh!

github-actions bot commented Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

Uh oh!

lorentzenchr commented Mar 26, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

fcharras commented Mar 27, 2024

Uh oh!

betatim Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

ogrisel Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ogrisel commented Apr 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

betatim commented Apr 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

fcharras commented Nov 17, 2023 •

edited by ogrisel

Loading

github-actions bot commented Nov 17, 2023 •

edited

Loading

ogrisel commented Apr 24, 2024 •

edited

Loading