FIX: fix array API support for float32-only devices on integer inputs#33898
FIX: fix array API support for float32-only devices on integer inputs#33898cakedev0 wants to merge 49 commits into
Conversation
….intel.com instead
This reverts commit 0837fe9.
…annot expect dtype objects to be hashable
…ted on the main CI
cakedev0
changed the title
dpnp testsdpnp tests
| max_dtype = _max_precision_float_dtype(xp, device=device) | ||
| # Also test with integer input only once per namespace/device for | ||
| # namespaces that support integer-by-floating matmul. | ||
| if X_xp.dtype != max_dtype or array_namespace in {"array_api_strict", "torch"}: | ||
| return | ||
|
|
||
| X_int = (X * 10).astype(np.int64) | ||
| atol *= 10 | ||
| X_xp = xp.asarray(X_int, device=device) | ||
| with config_context(array_api_dispatch=True): | ||
| Q_np = randomized_range_finder(X_int, size=size, n_iter=n_iter, random_state=0) | ||
| Q_xp = randomized_range_finder(X_xp, size=size, n_iter=n_iter, random_state=0) | ||
|
|
||
| assert get_namespace(Q_xp)[0].__name__ == xp.__name__ | ||
| assert Q_xp.dtype == max_dtype | ||
| assert_allclose(move_to(Q_xp, xp=np, device="cpu"), Q_np, atol=atol) |
There was a problem hiding this comment.
This is maybe out-of-scope for this PR: it checks for a bug that happened in my first iteration of this PR if X was an int-array (which is allowed but probably fairly uncommon). This case was not covered by tests, so I added this piece of test. I'm ok to move it in a separate PR.
There was a problem hiding this comment.
I think it's ok to keep it here.
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
Could you please try to find a way to add a few non regression tests (e.g. for the code that previously failed to pass the device to supported_float_dtypes)? I suppose this can be achieved by adding a few test cases to run on integer array API inputs using the MPS device on macOS as it does not support float64.
We could add a TODO comment to later change those tests to use array-api-strict instead once released with data-apis/array-api-strict#206.
ogrisel
changed the title
dpnp tests
|
I changed the title to make reflect the actual scope of this fix. Please also add a changelog entry (once the non-regression tests are in). |
|
Added non-reg tests and changelog. You can see that they are indeed failing before the fixes from this PR: https://github.com/scikit-learn/scikit-learn/actions/runs/25725657860/job/75537790919?pr=33997 |
ogrisel
added
Waiting for Second Reviewer
3 participants
Reference Issues/PRs
Follow-up from #32460 and #33895
What does this implement/fix? Explain your changes.
This PR mostly passes around
device=device. Mostly insupported_float_dtypesfunction. Withdpnpthe default device is the GPU, and on my laptop the GPU doesn't support float64, sosupported_float_dtypescalled without passing the CPU device doesn't return float64 and this turns into test failures.This PR also adapt how a float64 gaussian noise numpy-array is transferred from numpy to
xpin_randomized_range_finder, for similar reasons than above.With this PR, all dpnp tests pass on my machine (GPU and CPU), as proven by this CI pipeline (see issue: https://github.com/probabl-ai/scikit-learn-intel-workflow/actions/runs/25327052888/job/74250515053