Skip to content
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

Fix arange issues #945

Merged
merged 2 commits into from
Oct 20, 2022
Merged

Fix arange issues #945

merged 2 commits into from
Oct 20, 2022

Conversation

oleksandr-pavlyk
Copy link
Collaborator

This PR fixes several unexpected behaviors exposed when using tensor.arange in dpnp.

  • Issue 1:
# used to raise an exception, now works
dpt.arange(0, stop=10, step=None)
  • Issue 2:
# Used to return an empty array, expected array of length 1
dpt.arange(9.7, stop=10)
  • Issue 3:
# Used to raise and exception, now produces Boolean array of length, same as NumPy
dpt.arange(0, stop=2, dtype='bool')
  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?

@oleksandr-pavlyk oleksandr-pavlyk requested review from ndgrigorian, npolina4 and antonwolfy and removed request for ndgrigorian October 18, 2022 18:52
@github-actions
Copy link

@coveralls
Copy link
Collaborator

coveralls commented Oct 18, 2022

Coverage Status

Coverage decreased (-0.03%) to 82.102% when pulling 2e78bbb on fix-arange-issues into 59980a2 on master.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_195 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_197 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@antonwolfy
Copy link
Collaborator

The below example isn't aligned with numpy.arange():

numpy.arange(9.7, stop=200, step=100, dtype=numpy.int32)
# Out: array([  9, 109], dtype=int32)

dpctl.tensor.asnumpy(dpctl.tensor.arange(9.7, stop=200, step=100, dtype=numpy.int32))
# Out: array([  9, 108], dtype=int32)

@oleksandr-pavlyk
Copy link
Collaborator Author

The below example isn't aligned with numpy.arange():

numpy.arange(9.7, stop=200, step=100, dtype=numpy.int32)
# Out: array([  9, 109], dtype=int32)

dpctl.tensor.asnumpy(dpctl.tensor.arange(9.7, stop=200, step=100, dtype=numpy.int32))
# Out: array([  9, 108], dtype=int32)

Should be fixed now, please recheck.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_198 ran successfully.
Passed: 32
Failed: 802
Skipped: 3138

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_199 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_200 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

antonwolfy
antonwolfy previously approved these changes Oct 19, 2022
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_201 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@antonwolfy
Copy link
Collaborator

dpnp tests are passing now

… dpnp

dpt.arange(0, stop=10, step=None) raised, works in numpy
dpt.arange(9.7, stop=10) gave empty array, gives 1 element array in numpy
dpt.arange(0,stop=2, dtype='bool') raised, works in numpy

First two were just bugs, and got fixed.

The last one now works through special-casing bools. It works by
constructing int8 temporary and casting it into bool array only if
the resulting sequence has length 0, 1, or 2.

Aligned with behavior of np.arange in computation of the step.

To this end changed the logic of determining
step argument for the call to `_linspace_step` routine. We now
compute first and second element of the array of given type, and
determine the step as a the difference of these.

To avoid possible overflow message when subtracting unsigned integers,
cast first and second element to int64, subtract, and cast to the
target type.
Fixed test_arange and added test_arange_mixed_dtype

The test_arange test was specifying out of bounds starting value
for certain types.

Added tests to based on examples highlighting discrepancies.

dpt.arange(-2.5, stop=200, step=100, dtype='int32') now produces
the sequence consistent with np.arange output.

So does dpt.arange(9.7, stop=200, step=100, dtype='i4')

```ipython
In [1]: import dpctl, dpctl.tensor as dpt, numpy as np

In [2]: dpt.asnumpy(dpt.arange(9.7, stop=200, step=100, dtype='i4'))
Out[2]: array([  9, 109], dtype=int32)

In [3]: np.arange(9.7, stop=200, step=100, dtype='i4')
Out[3]: array([  9, 109], dtype=int32)
```

```
ty = np.float32
assert (
   dpt.arange(
       ty(0), stop=ty(504.0), step=ty(100), dtype=ty
   ).shape == (5,)
)
```

```
In [4]: import numpy as np

In [5]: np.arange(-5, stop=10**5, step=2.7, dtype=np.int64).shape
Out[5]: (37039,)

In [6]: import dpctl.tensor as dpt

In [7]: dpt.arange(-5, stop=10**5, step=2.7, dtype=np.int64).shape
Out[7]: (50003,)
```
@oleksandr-pavlyk
Copy link
Collaborator Author

This PR also fixed other two issues related to computation of output length :

_stop = np.float32(504)
x = dpt.arange(0, stop=_stop, step=100, dtype="f4", device=x.device)
assert x.shape == (6,)

# ensure length is determined using uncast parameters
x = dpt.arange(-5, stop=10**2, step=2.7, dtype="int64", device=x.device)
assert x.shape == (39,)

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_195 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

@oleksandr-pavlyk oleksandr-pavlyk merged commit 2723947 into master Oct 20, 2022
@oleksandr-pavlyk oleksandr-pavlyk deleted the fix-arange-issues branch October 20, 2022 14:41
@github-actions
Copy link

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_195 ran successfully.
Passed: 33
Failed: 801
Skipped: 3138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants