Conversation
| assert not is_writeable_array(x) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("library", all_libraries) |
| # Unknown Array API compatible object. Note that this test may have dire consequences | ||
| # in terms of performance, e.g. for a lazy object that eagerly computes the graph | ||
| # on __bool__ (dask is one such example, which however is special-cased above). |
There was a problem hiding this comment.
This is a good argument for replacing everything below this point with a blind return False. Please discuss if you'd prefer it that way.
There was a problem hiding this comment.
This seems fine as is to me, since it's more correct. In case it's a problem for a library that's not explicitly handled, then support for it can be added, or a discussion can happen then - at least we'll learn something.
array_api_compat/common/_helpers.py
Outdated
| if ( | ||
| is_numpy_array(x) | ||
| or is_cupy_array(x) | ||
| or is_torch_array(x) |
There was a problem hiding this comment.
FIXME is this correct and safe? I don't know enough about torch.compile myself.
There was a problem hiding this comment.
It works, at least for most backends, since TorchDynamo can handle graph breaks. If we have to make a choice here, then I'd say what you wrote is correct for PyTorch.
crusaderky
force-pushed
the
is_lazy_array
branch
from
82fab45 to
b235ff4
Compare
| # Unknown Array API compatible object. Note that this test may have dire consequences | ||
| # in terms of performance, e.g. for a lazy object that eagerly computes the graph | ||
| # on __bool__ (dask is one such example, which however is special-cased above). | ||
|
|
There was a problem hiding this comment.
array_api_strict reaches this point
crusaderky
force-pushed
the
is_lazy_array
branch
from
2601391 to
f3ed389
Compare
crusaderky
force-pushed
the
is_lazy_array
branch
from
f3ed389 to
a3412e4
Compare
crusaderky
force-pushed
the
is_lazy_array
branch
from
de95390 to
7950eaa
Compare
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
LGTM, thanks @crusaderky. And is_lazy_array seems okay as a helper function in this library rather than in array-api-extra.
@ev-br @asmeurer @lucascolley any of you want to take a look?
ev-br
left a comment
ev-br
left a comment
There was a problem hiding this comment.
Let's roll with this indeed, if only to unblock the jax work. FWIW I'm not sure about the "unknown library" dance, it looks complicated and potentially costly (as discussed in the comments), but that's something we'll be able to adjust en route. Thanks @crusaderky, all.
5 participants
Closes #225