-
Notifications
You must be signed in to change notification settings - Fork 46
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
Python scalars in elementwise functions #807
Comments
Hmm,I am in two minds about reconsidering this choice. On the con side: non-array input to functions is going against the design we have had from the start, it makes static typing a bit harder (we'd need both an >>> import torch
>>> t = torch.ones(3)
>>> torch.maximum(t, 1.5)
...
TypeError: maximum(): argument 'other' (position 2) must be Tensor, not float In principle PyTorch is fine with adding this it looks like, but it's a nontrivial amount of work and no one is working on it as far as I know: pytorch/pytorch#110636. PyTorch does support it in functions matching operators (e.g., TensorFlow also doesn't support it (except for in their
The readability argument is less prominent for functions that for operators though. Both because On the pro side: I agree that it is pretty annoying to get right in a completely portable and generic way. In the cases where one does need it, the natural choice of xp.maximum(x, xp.asarray(1, dtype=x.dtype, device=x.device)) Hence if this is a pattern that a project happens to need a lot, it will probably create a utility function like: def as_zerodim(value, x, /, xp=None):
if xp is None:
xp = array_namespace(x)
return xp.asarray(value, dtype=x.dtype, device=x.device)
# Usage:
xp.maximum(x, as_zerodim(1, x)) PyTorch support comes through |
We could support them in a bespoke way for specific useful functions' arguments like |
It's even a little messier in the case Xarray is currently facing:
|
That's deviating from even the operator behavior in the array API. The specified Similarly, |
This is for consistency with operators, which allow combining an int with an array that has a floating-point data type. See the discussion at data-apis#807.
List of potential APIs to support Python scalars in:
I've omitted all 1-argument elementwise functions. Also these multi-argument elementwise functions, which seem less useful at a glance, but let me know if adding scalar support to any of these would be useful:
By the way, |
Thanks Aaron. I agree with the choices here:
Since there must always be at least one input that is an array, it would be more annoying to statically type if both arguments may be scalar. It then requires overloads, just an |
Do you agree that allowing scalars for both arguments of If so, should we allow scalars in both arguments? It could be useful, for instance, to just compute a specific float |
Just wanted to add a +1 to this effort; it would really simplify translation efforts. If you need additional opinions about fine points, LMK. |
One consideration that came up in discussion: How can users write new elementwise functions that support scalars in some arguments themselves using the array API? e.g., suppose we want def polar_to_cartesian(r, theta):
xp = get_array_namespace(r, theta)
return (r * xp.sin(theta), r * xp.cos(theta)) This seems to require supporting scalars even in single-argument elementwise operations like |
@shoyer Your example may run into issues due to device selection. What device should libraries supporting multiple devices (e.g., PyTorch) allocate the returned arrays from |
I find it a bit weird to not allow the scalar in both (would be nice if you could just add an Only allowing one of the two is a bit strange for assymetric functions. |
Ditto for Based on the discussions in the meeting today, we should still require at least one argument to be an array for now (or rather, leave full scalar inputs unspecified), but I don't see any reason to prefer arguments 1 or 2 for Not including scalar support for argument 1 in |
The brief summary of that was:
|
There are also some functions I intentionally omitted from my list above because it doesn't really seem to make sense for them to include scalars, even though they support multiple arrays since they are array manipulation functions, like |
I can maybe see a case for supporting scalars in Scalars don't make sense in |
Do we want to reboot this topic? Via "multi device" support in array-api-strict and the scikit-learn tests we found scipy/scipy#21736 - which I think will be a common bug without support for scalars. If only because it is quite tedious to spell out the correct version each time. I am motivated to get this done and implemented in array-api-compat. Maybe we can only do it for those functions that we can agree on/where it is a "no brainer"? Then do a second pass later for other functions? |
We basically agreed to do this, so it just needs updates for the standard. We can implement things in compat even if they are only in a draft release of the standard. If anything it's better to implement things first because then we can catch potential issues. |
Do you want to make the change in compat or should I try? |
If you want to make the change feel free. If you don't, I will, but I also have other things that I'm working on presently so I might not get to it until later this week or next week. |
The array API supports Python scalars in arithmetic only, i.e., operations like
x + 1
.For the same readability reasons that supporting scalars in arithmetic is valuable, it would nice to also support Python scalars in other elementwise functions, at least those that take multiple arguments like
maximum(x, 0)
orwhere(y, x, 0)
.The text was updated successfully, but these errors were encountered: