ENH: add fallback_namespace #39

tupui · 2023-04-26T15:56:25Z

It was suggested in tupui/scipy#24 that we could add a default namespace as a fallback.

This would allow to do the following:

array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=array_api_compat.np)

Internally [1, 2] is evaluated for 1 and 2 and in both cases it will fallback to array_api_compat.np instead of raising. (Note that I am not sure about the use case for the list/tuple as one can do array_namespace(*x))

rgommers · 2023-04-26T18:08:15Z

fallback_namespace=xp

This seems a little misleading, because xp is typically used as the return value for the generic namespace - so it does not have a default. Your test case make it clear what you want, and that is fine. So I suggest spelling it everywhere as fallback_namespace=array_api_compat.np.

I'm also not 100% sure if array coercion and namespace selection should be mixed. @asmeurer was that what you intended with your suggestion?

asmeurer · 2023-04-26T20:36:09Z

tests/test_array_namespace.py

+    xp_ = array_namespace([1, 2], fallback_namespace=xp)
+    assert xp_ == xp
+
+    xp_ = array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=xp)


Can we add a test to make sure the fallback namespace only applies when it's ambiguous. Something like array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=numpy.array_api) should return array_api_compat.numpy.

I am not sure how this would work. To me that's the purpose of the fallback. With

array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=numpy.array_api)

I would expect [1, 2] to fallback to numpy.array_api. I suppose that in this case we could say ok fallback [1, 2] to be the same as the other namespace. But what if instead we have:

array_namespace([1, 2], cp.asarray([1, 2]), fallback_namespace=numpy.array_api)

Shall this also simplify to cp??

(Due to the recursions it's also getting tricky to follow the logic.)

Maybe I'm missing the point of this then. In general I'd say you shouldn't mix different array libraries with the array API. Some libraries might support it (like NumPy being able to convert other arrays), and in some cases with a performance penalty (like moving from GPU to CPU). So array_namespace([1, 2], cp.asarray([1, 2]), fallback_namespace=numpy.array_api) should give an error.

So to me the only point of this is to allow this function to also accept non-array inputs, like Python scalars and/or lists of Python scalars. You can convert those to arrays with asarray, but you can't call asarray until you have a namespace, making it a chicken/egg situation. Of course, then it might still be a good idea to make sure the errors from this function are helpful.

I also tend to agree that the best way to handle this is to simply not allow non-array inputs to functions that use the array API. That forces the users to resolve the ambiguity of what array library the want to use by calling asarray themselves.

By the way, this function automatically denests lists of arrays. This was to internally support functions like concat which accept lists of arrays. But I'm wondering if this behavior should be removed, so that there there is less ambiguity and we can unconditionally error on list or tuple input.

Yeah the main purpose of the proposal was to assume a namespace for things that would otherwise be marked as non compatible or unknown (list, scalar, tuples, else). Then yes it's on the user to call asarray and whether or not it works is on their side.

By the way, this function automatically denests lists of arrays.

Yeah this is making the logic difficult IMHO. I was having trouble to find a way to address your comment with that in place. If that can be removed, I am happy to do so (here or another PR, let me know.)

Yeah the main purpose of the proposal was to assume a namespace for things that would otherwise be marked as non compatible or unknown (list, scalar, tuples, else).

I think that should live in SciPy, because it's orthogonal to array API support. Use of lists and other "array-likes" is specific to NumPy, and is by and large considered a design mistake. The array API standard does not allow array-likes, and neither do other libraries like PyTorch and CuPy. So there's no reason for this package to care about array-likes.

If that can be removed, I am happy to do so (here or another PR, let me know.)

If you can do that it would be great. I'm still unclear what the changes here are (do we actually want this or not?), so maybe a separate PR is appropriate. You'll need to fix the functions like concat that do accept a list. Right now the wrappers assume they can just pass things through to array_namespace.

I think that should live in SciPy, because it's orthogonal to array API support. Use of lists and other "array-likes" is specific to NumPy, and is by and large considered a design mistake. The array API standard does not allow array-likes, and neither do other libraries like PyTorch and CuPy. So there's no reason for this package to care about array-likes.

I agree. For functions that are wrapped functions from NumPy (or any other library), we should support whatever the wrapped function supports to maintain maximal compatibility. But for new functions, we can be a little more strict. And generally speaking array_namespace() is an important function that will be used everywhere so we should try to discourage bad habits with it.

Ok then it looks like we could close this PR and I could try to propose a PR to only accept Array API compatible arrays. Is that correct?

Yes that sounds good.

asmeurer · 2023-04-26T20:37:29Z

array_api_compat/common/_helpers.py

@@ -53,7 +53,7 @@ def _check_api_version(api_version):
    if api_version is not None and api_version != '2021.12':
        raise ValueError("Only the 2021.12 version of the array API specification is currently supported")

-def array_namespace(*xs, api_version=None, _use_compat=True):
+def array_namespace(*xs, api_version=None, _use_compat=True, fallback_namespace=None):


The fallback namespace should still be an array API compatible namespace. So if someone does fallback_namespace=numpy instead of fallback_namespace=array_api_compat.numpy, we might want to automatically convert for them.

I added some logic to try to convert the namespace. I did not see a function to do that but I suppose this could be made into it's own function if there are more needs.

asmeurer · 2023-04-26T20:39:11Z

Looks like the test failures in the CI are unrelated. You can ignore the "Array API Tests" jobs for this PR and only worry about the "Tests" jobs, because this function won't affect the array API testsuite.

tupui

Thanks @asmeurer! I did some updates based on your comments.

tupui · 2023-04-27T11:56:12Z

array_api_compat/common/_helpers.py

@@ -53,7 +53,7 @@ def _check_api_version(api_version):
    if api_version is not None and api_version != '2021.12':
        raise ValueError("Only the 2021.12 version of the array API specification is currently supported")

-def array_namespace(*xs, api_version=None, _use_compat=True):
+def array_namespace(*xs, api_version=None, _use_compat=True, fallback_namespace=None):


I added some logic to try to convert the namespace. I did not see a function to do that but I suppose this could be made into it's own function if there are more needs.

tupui · 2023-04-27T11:59:31Z

tests/test_array_namespace.py

+    xp_ = array_namespace([1, 2], fallback_namespace=xp)
+    assert xp_ == xp
+
+    xp_ = array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=xp)


I am not sure how this would work. To me that's the purpose of the fallback. With

array_namespace([1, 2], np.asarray([1, 2]), fallback_namespace=numpy.array_api)

I would expect [1, 2] to fallback to numpy.array_api. I suppose that in this case we could say ok fallback [1, 2] to be the same as the other namespace. But what if instead we have:

array_namespace([1, 2], cp.asarray([1, 2]), fallback_namespace=numpy.array_api)

Shall this also simplify to cp??

(Due to the recursions it's also getting tricky to follow the logic.)

thomasjpfan · 2023-04-27T13:29:58Z

In scikit-learn, we required that the input must be an array to use Array API. Specifically, the input must be an array that adopts the Array API spec or arrays extended by array_api_compat (torch, cupy).

If the input was a list and scikit-learn's Array API config switch was on, then the failure is expected.

rgommers · 2023-04-27T13:49:30Z

If the input was a list and scikit-learn's Array API config switch was on, then the failure is expected.

+1 for that, I'd like SciPy to do the same.

tupui · 2023-04-27T14:06:46Z

Oh ok, then that's pretty much the default behaviour of array_api_compat.array_namespace. I will make the changes 👍

tupui added 2 commits April 26, 2023 17:48

ENH: add fallback_namespace

87eec29

TST: test fallback and multiple namespaces due to fallback

3b90238

tupui mentioned this pull request Apr 26, 2023

ENH: asarray for Array API support tupui/scipy#24

Closed

asmeurer reviewed Apr 26, 2023

View reviewed changes

ENH: add conversion to Array API compatible namespace

0acc2b0

tupui commented Apr 27, 2023

View reviewed changes

tupui closed this May 1, 2023

tupui mentioned this pull request May 2, 2023

ENH: disallow passing array-like to array_namespace #42

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: add fallback_namespace #39

ENH: add fallback_namespace #39

tupui commented Apr 26, 2023 •

edited

Loading

rgommers commented Apr 26, 2023

asmeurer Apr 26, 2023

tupui Apr 27, 2023

asmeurer Apr 27, 2023

asmeurer Apr 27, 2023

tupui Apr 27, 2023

rgommers Apr 27, 2023

asmeurer Apr 27, 2023

asmeurer Apr 27, 2023

tupui Apr 28, 2023

asmeurer Apr 28, 2023

asmeurer Apr 26, 2023

tupui Apr 27, 2023

asmeurer commented Apr 26, 2023

tupui left a comment

tupui Apr 27, 2023

tupui Apr 27, 2023

thomasjpfan commented Apr 27, 2023

rgommers commented Apr 27, 2023

tupui commented Apr 27, 2023

ENH: add fallback_namespace #39

ENH: add fallback_namespace #39

Conversation

tupui commented Apr 26, 2023 • edited Loading

rgommers commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmeurer commented Apr 26, 2023

tupui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasjpfan commented Apr 27, 2023

rgommers commented Apr 27, 2023

tupui commented Apr 27, 2023

tupui commented Apr 26, 2023 •

edited

Loading