Skip to content

Type promotion tests #2

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

Closed
asmeurer opened this issue Aug 28, 2020 · 25 comments
Closed

Type promotion tests #2

asmeurer opened this issue Aug 28, 2020 · 25 comments

Comments

@asmeurer
Copy link
Member

Here is the spec for type promotion rules: https://github.com/data-apis/array-api/blob/master/spec/API_specification/type_promotion.md

Questions:

  • As with Broadcasting tests #1, how do I create inputs for the tests? I didn't see any specification on how to create arrays or how to specify dtypes.

  • Do I read the document correctly in that all the listed dtypes are required to be implemented?

  • The bullet points at the bottom specify different semantics for zero-dimensional arrays and scalars. Is the distinction between these two spelled out somewhere? As far as I know, NumPy distinguishes these types but they mostly behave the same (and they seem to both participate in type promotion).

@rgommers
Copy link
Member

  • As with Broadcasting tests #1, how do I create inputs for the tests? I didn't see any specification on how to create arrays or how to specify dtypes.

Yes we need to write those sections still. You can assume numpy names for dtypes and array creation functions (don't use np.random ones though, random number generation may not be in the core).

Do I read the document correctly in that all the listed dtypes are required to be implemented?

Yes indeed. That's why the set of dtypes is quite conservative - all libraries have the listed dtypes.

The bullet points at the bottom specify different semantics for zero-dimensional arrays and scalars. Is the distinction between these two spelled out somewhere? As far as I know, NumPy distinguishes these types but they mostly behave the same (and they seem to both participate in type promotion).

I think those two bullet points are a complete specification (the first one just emphasizes that 0-D arrays are not special - they're just arrays). NumPy's rules are much more complicated. Scalars in the spec are Python scalars, the NumPy "array scalars" are not a thing.

@asmeurer
Copy link
Member Author

Scalars in the spec are Python scalars

OK, that's a bit confusing, especially since Python ints don't fit any of the given dtypes. I would use the word "native Python types" rather than scalars.

And just to clarify then, what is the spec behavior on Python scalars? Is it implementation dependent? Should a Python float always cast like a float64? Should int cast to some int type, assuming it is small enough? The bullet point seems to imply that they should give an error in all cases. Is that really intended?

@rgommers
Copy link
Member

rgommers commented Aug 28, 2020

And just to clarify then, what is the spec behavior on Python scalars? Is it implementation dependent? Should a Python float always cast like a float64? Should int cast to some int type, assuming it is small enough? The bullet point seems to imply that they should give an error in all cases. Is that really intended?

Native types should never upcast. That is consistent with what libraries do now I believe. (EDIT: modulo the "no mixed types below)

The main thing that isn't specified well is what happens for dtypes not present in the table. Most importantly, mixed float/int tables are left out. Those are all inconsistently implemented: TensorFlow raises, PyTorch has (e.g.) float32 + int64 -> float32, NumPy is more aggressive in upcasting and returns float64. Hence it was decided to disallow that - raise an exception and make the user do an explicit cast.

@kgryte we have that discussion in the meeting minutes, but I'm thinking we should add both the decision and the rationale to the spec, maybe in a note box that can fold out (the theme has those). WDYT?

@kgryte
Copy link

kgryte commented Aug 31, 2020

@rgommers I don't know if we ever came to a decision regarding dtypes which are not specified in the tables. Currently, it is implementation-dependent, but I cannot recall whether we came to consensus on whether, e.g., this should remain the case or whether these situations should be "disallowed"/trigger an exception.

Regardless, agreed that we should add a section explaining the rationale for not including every combination and why we have not yet sought to standardize behavior in this particular circumstance.

@kgryte
Copy link

kgryte commented Aug 31, 2020

@asmeurer See here for likely array creation APIs. I should have a PR up for these within the next day or so.

@rgommers
Copy link
Member

@rgommers I don't know if we ever came to a decision regarding dtypes which are not specified in the tables. Currently, it is implementation-dependent, but I cannot recall whether we came to consensus on whether, e.g., this should remain the case or whether these situations should be "disallowed"/trigger an exception.

Yes you are totally right, don't know what I was thinking when I wrote that. If we say "must raise an exception", that is standardizing it.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 1, 2020

What's the best way to create empty arrays (arrays with 0 in their shape)? empty isn't on that list. Should it just be array([]).reshape(shape)? It would be nice if array([], shape=shape) worked, but it doesn't in NumPy (numpy/numpy#16665) and I don't know if it does in any of the other libraries.

@rgommers
Copy link
Member

rgommers commented Sep 1, 2020

np.ones((4,2,0), dtype=np.float64) should do it.

@rgommers
Copy link
Member

rgommers commented Sep 1, 2020

Interesting that empty isn't on it, I had expected it to be. It may end up in the spec anyway, but for this purpose it doesn't matter, ones, zeros, or full all do the same thing if one dimension is size 0.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 1, 2020

Native types should never upcast. That is consistent with what libraries do now I believe. (EDIT: modulo the "no mixed types below)

I guess I'm still very confused by what is meant here. As far as I can tell NumPy treats int the same as int64 and float the same as float64:

>>> 1 + np.int64(1)
2
>>> type(1 + np.int16(1))
<class 'numpy.int64'>
>>> type(1 + np.int8(1))
<class 'numpy.int64'>
>>> type(1 + np.int16(1))
<class 'numpy.int64'>
>>> type(1 + np.int32(1))
<class 'numpy.int64'>
>>> type(1 + np.int64(1))
<class 'numpy.int64'>
>>> 1.0 + np.float64(1.0)
2.0
>>> type(1.0 + np.float32(1.0))
<class 'numpy.float64'>
>>> type(1.0 + np.float64(1.0))
<class 'numpy.float64'>

If the int doesn't fit in the type it gives an int64, and if it doesn't fit in that it gives an int:

>>> 2**10 + np.int8(1)
1025
>>> type(2**10 + np.int8(1))
<class 'numpy.int64'>
>>> 2**100 + np.int64(1)
1267650600228229401496703205377
>>> type(2**100 + np.int64(1))
<class 'int'>

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

Those are all "scalar + scalar". I thought you meant "scalar + array", which is the interesting case.

This type of thing, np.float64(1.0), gives array scalars - those do not exist in other array libraries, and are largely considered a mistake for numpy. So they won't be in the spec, no need to worry about them.

Example:

In [2]: a = torch.float64(1.5)                                                
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-d09d5b5a50e6> in <module>
----> 1 a = torch.float64(1.5)

TypeError: 'torch.dtype' object is not callable

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

So all there is is Python int/float/bool, and arrays with the dtypes that are in scope.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 2, 2020

Are shape () arrays not part of the spec? I didn't see anything about that. In NumPy shape () arrays seem to behave the same as "array scalars" in my examples, with the exception of 2**10 + np.ones((), dtype=np.int64) which gives a result of 1025 of dtype int64. My understanding was that shape () arrays basically are scalar arrays, and should more or less behave the same as "dtype scalars" in NumPy, and in the few places there are differences, the spec should focus on the shape () array behavior. From what I can tell, int + array acts like int64(int) + array and float + array acts like float64(float) + array.

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

Are shape () arrays not part of the spec?

Yes they are, but those are different from your examples. You'd construct them with np.array(1.5, dtype=np.float64), and they'd have type ndarray rather than a dtype instance.

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

They should have casting behavior identical to arrays with different shapes.

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

So this is all wrong in NumPy currently:

In [11]: x = np.array(1.5, dtype=np.int32)                                    

In [12]: x + 1.5                                                              
Out[12]: 2.5

In [13]: type((x + 1.5))                                                      
Out[13]: numpy.float64

That should give back an ndarray of shape ()

@asmeurer
Copy link
Member Author

asmeurer commented Sep 2, 2020

ones(()) seems to be returning that:

>>> type(np.ones((), dtype=np.int8))
<class 'numpy.ndarray'>

That should give back an ndarray of shape ()

According to the spec, it should raise an exception.

Or at least that's how I read it. I hope I've convinced you that the spec is extremely ambiguous here: "Non-array ("scalar") operands are not permitted to participate in type promotion." "Scalar" can mean one of three things (Python built-in, "dtype scalar", or shape () array). Nothing is defined anywhere. It doesn't even clarify what "not permitted" means. Should it be an exception? If "scalar" does mean Python built-in, what are the assumed dtypes of int, float, and bool (int in particular doesn't match any of the dtypes listed in the document). And the previous sentence also talking about zero-dimensional arrays but saying the opposite thing is even more confusing.

@rgommers
Copy link
Member

rgommers commented Sep 2, 2020

I hope I've convinced you that the spec is extremely ambiguous here:

Yes I agree. Making it concrete ("scalar means Python builtin types, float, int, bool, etc.") would help the reader and leave less room for misinterpretation.

Also for array scalars, we can note that they are left out, with rationale.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 4, 2020

I've pushed a proof of concept for type promotion here https://github.com/Quansight/array-api-tests/blob/master/array_api_tests/test_type_promotion.py#L74. It's not actually parameterized properly yet, and doesn't generate random array data.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 4, 2020

Actually it looks like dtype isn't in the list data-apis/array-api#6 (comment). So how should I write

a = array(..., dtype=dtype('i2'))
assert a.dtype == dtype('i2')

according to the spec? Should I use something like array([], dtype='i2').dtype instead of dtype('i2')? Or will dtype objects be added to the spec? Or should we use dtype literals like np.float32, etc.?

Also, just looking at some of the libraries, I'm a bit confused what the API namespace is, e.g., for tensorflow or pytorch. tensorflow.array doesn't exist, and neither does torch.array. Am I looking at the wrong namespace for these libraries, or array not really something that exists everywhere? I can probably build example arrays out of zeros, which seems to exist everywhere, although tensorflow.zeros and torch.zeros don't accept string dtype arguments. Is it one of those things that should exist according to the spec but they may not actually be compatible yet? Without this very basic thing (ability to create example arrays), none of the other tests can work, except for very basic namespace/signature tests.

@rgommers
Copy link
Member

rgommers commented Sep 5, 2020

Actually it looks like dtype isn't in the list data-apis/array-api#6 (comment). So how should I write

a = array(..., dtype=dtype('i2'))
assert a.dtype == dtype('i2')

according to the spec? Should I use something like array([], dtype='i2').dtype instead of dtype('i2')? Or will dtype objects be added to the spec? Or should we use dtype literals like np.float32, etc.?

That's also a section that still needs to be added, but I'm almost certain we should only use dtype literals. The dtype constructor and dtype instances indeed aren't common between libraries. Quick example:

In [1]: import torch                                                          

In [2]: torch.dtype('i2')                                                     
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-1ebb90475621> in <module>
----> 1 torch.dtype('i2')

TypeError: cannot create 'torch.dtype' instances

I think float32 is also much more explicit and easier to understand than dtype('f4').

Also, just looking at some of the libraries, I'm a bit confused what the API namespace is, e.g., for tensorflow or pytorch. tensorflow.array doesn't exist, and neither does torch.array. Am I looking at the wrong namespace for these libraries, or array not really something that exists everywhere?

Namespaces are all not lined up well. Given both that issue, and that there will be BC-breaking things in the standard for every library, the adoption plan will be to add some new namespace or function (e.g. `mod = get_array_module(api_version='1.0') will give you a namespace with all the standard-compliant names and behaviour).

And yes, array doesn't exist everywhere but should be introduced. Some libraries have a tensor() constructor or something else; I imagine that's one of the few cases where we may come to an agreement on an alias, where both tensor and array are present and do the same thing.

I can probably build example arrays out of zeros, which seems to exist everywhere, although tensorflow.zeros and torch.zeros don't accept string dtype arguments. Is it one of those things that should exist according to the spec but they may not actually be compatible yet? Without this very basic thing (ability to create example arrays), none of the other tests can work, except for very basic namespace/signature tests.

Agreed. I'd say you should go with array([1, 2, 3], dtype=amod.float64). And annoyingly, we have decided on the import convention yet. I wrote amod for "array module" here. That's not optimal (good place to bikeshed), but better than using np I believe.

@rgommers
Copy link
Member

rgommers commented Sep 5, 2020

I may write up something today for the dtypes and import convention, so we get that sorted out. And Athan is working on the array creation and manipulation functions I believe.

Thanks for pointing out all the holes you find @asmeurer, very helpful in prioritizing what to write and find pieces that we forgot about.

@asmeurer
Copy link
Member Author

asmeurer commented Sep 5, 2020

That's also a section that still needs to be added, but I'm almost certain we should only use dtype literals.

In that case, shouldn't the type promotion document be written in terms of them rather than the string shorthands?

Also mxnet doesn't seem to have them.

@rgommers
Copy link
Member

rgommers commented Sep 8, 2020

Clarification on dtype usage done in data-apis/array-api#32.

I think this should make clear that your example should be written as:

a = array(..., dtype=int16)
assert a.dtype == int16

Note that the "array object" section is still TODO, but it's a given that that will have a dtype attribute.

@asmeurer
Copy link
Member Author

asmeurer commented Oct 4, 2021

Type promotion is now tested extensively for elementwise functions and operators. All that remains to do is tests for the non-elementwise functions that participate in type promotion. I will open a new issue for this.

@asmeurer asmeurer closed this as completed Oct 4, 2021
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

No branches or pull requests

3 participants