-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: is_lazy_array() #228
base: main
Are you sure you want to change the base?
ENH: is_lazy_array() #228
Conversation
tests/test_common.py
Outdated
@@ -92,6 +93,70 @@ def test_is_writeable_array_numpy(): | |||
assert not is_writeable_array(x) | |||
|
|||
|
|||
@pytest.mark.parametrize("library", all_libraries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIXME is this correct and safe? I don't know enough about torch.compile myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array_api_strict reaches this point
2601391
to
f3ed389
Compare
@pytest.mark.parametrize("shape", [(math.nan,), (1, math.nan), (None, ), (1, None)]) | ||
def test_is_lazy_array_nan_size(shape, monkeypatch): | ||
"""Test is_lazy_array() on an unknown Array API compliant object | ||
with NaN (like Dask) or None (like ndonnx) in its shape |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be simplified after #231
|
||
# Select a single point of the array | ||
s = size(x) | ||
if s is None or math.isnan(s): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be simplified after #231
f3ed389
to
a3412e4
Compare
Closes #225