FIX Fix device detection when array API dispatch is disabled #30454
Conversation
|
Oh well some failures with array-api-strict that need to be investigated ... |
|
About the error message for the # Test expected value is returned otherwise
array1 = Array("device")
array2 = Array("device")
assert array1.device == device(array1)
assert array1.device == device(array1, array2)
assert array1.device == device(array1, array1, array2)should be inside the |
|
Regarding the error message for the with config_context(array_api_dispatch=True):
_fill_or_add_to_diagonal(array_xp, value=1, xp=xp, add_value=False, wrap=wrap)
assert_array_equal(_convert_to_numpy(array_xp, xp=xp), array_np)It suppresses the error message, but I'm not entirely sure if this is the correct fix... |
|
CUDA CI passed on 0073b09 |
sklearn/utils/estimator_checks.py
Outdated
| # we end up doing X[train_indices] where X is a array-api-strict array | ||
| # and indices a numpy array. Probably not worth investigating for now, | ||
| # since using array-api-strict with array API disabled does not seem a | ||
| # very relevant. |
There was a problem hiding this comment.
While I agree that we don't really care about supporting array-api-strict inputs when array API dispatch is disabled, I still think that those X[train_indices] are bugs in our code. We should probably always do X[xp.asarray(train_indices, device=device(X)] in our CV tools.
There was a problem hiding this comment.
I tried to tweak the comment a bit to be more self-explanatory. I am not sure it is so easy to tackle and discussing this in real-life this is probably not worth doing anything about it for now. All the array API library I could try (PyTorch, jax, CuPy) accept to index with numpy array.
For compleness, the proposed work-around would not work because the issue is that X is an array-api-strict array, train_indices is a numpy array and array-api-strict is insisting that to index X you need to pass array-api-strict array.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
There was a problem hiding this comment.
This looks good. Thanks @lesteve
Just some minor suggestions.
Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
…learn#30454) Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
Fix #29107, in particular #29107 (comment) which is a regression in 1.6.
Main change: with
array_api_dispatch=False, the device is alwaysNone(I tried"cpu"originally but this doesn't 8000 work with array-api-strict). In this casenp.asarraywill be called and the resulting array will always be CPU. We don't want to check devices too early and prevent thenp.asarrayconversion to happen (there may be an error like np.asarray with a PyTorch array on a CUDA device).Tests added:
estimator_checksthat makes sure that when array API is disabled you can fit and call other methods on numpy-convertible arrays (for example PyTorch array on CPU)