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

[Feature request] Support different out dtypes for elementwise func #1717

Open
vlad-perevezentsev opened this issue Jun 25, 2024 · 2 comments
Labels
question Further information is requested

Comments

@vlad-perevezentsev
Copy link
Collaborator

Current implementations of elementwise functions in dpctl.tensor do not support out data type other than bool and raise ValueError if out is not bool.

import dpctl.tensor as dpt

a = dpt.asarray([1,2,3])
out = dpt.empty_like(a,dtype='f4')
dpt.logical_not(a,out=out)

# ValueError: Output array of type bool is needed, got float32

While numpy can cast the result depending on out data type.

import numpy

a_np = dpt.asnumpy(a)
out_np = dpt.asnumpy(out)
numpy.logical_not(a_np, out_np)

# array([0., 0., 0.], dtype=float32)

Since the Python Array API has no description for out parameter we can implement any logic we want.
I think it would be useful to follow the logic of numpy and cast the result depending on out data type.

@ndgrigorian
Copy link
Collaborator

I would like to offer a correction that not only bool is supported, it depends on the expected output type of the elementwise functions.

logical_not expects to output a boolean array, so only accepts a boolean array as out. Something like sqrt expects to output floating-point data, so only accepts floating-point data type arrays for out. The reason for this being that for the output array of the function to actually be the same array as the out array, it needs to be the appropriate data type, since we can't cast the array data in-place. In such a case, the returned array would not be the same as the user input out.

@oleksandr-pavlyk
Copy link
Collaborator

The out keyword feature is intended to unlock memory reuse and optimize the code by avoiding allocations/deallocations of temporaries.

Supporting arbitrary elemental types of out array does not serve any of these purposes. Generically,with the suggested behavior binary_op(x1,x2,out=out) would be equivalent to out[...]=binary_op(x1,x2) which a user may do explicitly already.

I would rather prefer to raise an exception to bring the inefficiency to the forefront than do something inefficient and make the inefficiency harder to find.

I am voting to close as won't fix.

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

No branches or pull requests

3 participants