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

BUG: Architecture-inconsistent behaviour casting negative integers to unsigned integers (MacOS ARM) #22951

Closed
cmutel opened this issue Jan 5, 2023 · 4 comments
Labels

Comments

@cmutel
Copy link

cmutel commented Jan 5, 2023

Describe the issue:

Converting a negative integer with dtype float, float32, or float64 to uint32 or uint64 gives zero on MacOS ARM machines.

  • Tested on Numpy 1.23 and 1.24.
  • Tested on multiple MacOS ARM machines.
  • Expected behaviour is to wrap around the maximum unsigned integer, which is what is done on x64 machines
  • Wrap around also works when converting to uint8 and unint16

This exact operation is discussed in the 1.24 release notes, and the provided example does work. However, it fails when the input has dtype float, and conversion is to 32- or 64-bit unsigned integers.

Reproduce the code example:

import numpy as np
np.array(-1).astype(np.uint32)
>>> array(4294967295, dtype=uint32)
np.array(-1, dtype=float).astype(np.uint32)
>>> array(0, dtype=uint32)

Error message:

No response

Runtime information:

1.23.2
3.10.5 | packaged by conda-forge | (main, Jun 14 2022, 07:07:06) [Clang 13.0.1 ]

-and-

1.24.1
3.9.13 | packaged by conda-forge | (main, May 27 2022, 17:00:33)
[Clang 13.0.1 ]
[{'simd_extensions': {'baseline': ['NEON', 'NEON_FP16', 'NEON_VFPV4', 'ASIMD'],
'found': ['ASIMDHP', 'ASIMDDP'],
'not_found': ['ASIMDFHM']}}]

Context for the issue:

This breaks correct matrix building for the Brightway life cycle assessment framework.

@seberg
Copy link
Member

seberg commented Jan 5, 2023

NumPy inherits the C99 behavior, which is undefined for out-of-bounds float values when cast to integers.

That means, you sometimes get 0, sometimes you get the minimum value (even on the same system it can vary due to different intrinsics being used!). Or in same cases, the value is correctly converted to integer and you get normal integer overflow results (i.e. -1.0 becoming the maximum unsigned integer).

On NumPy 1.24.1 you should probably be seeing a warning ("invalid value") indicating undefined behavior.

NumPy could certainly clean this up to some degree. But some tries to speed up casting would more likely increase how often you get the undefined behavior.
It could be interesting to see if we can try to make the casts more safe than C guarantees without major slowdowns, but until someone dives into it, I am not sure there is much we can do. I.e. if you need to ensure out-of-bounds values are cast to something sane, you may have to remove the ahead of time.

@cmutel
Copy link
Author

cmutel commented Jan 6, 2023

@seberg Thanks a lot for your quick reply.

Stating that this is undefined behaviour is completely fine, and 1.24 does indeed add a warning, but the 1.24 release docs seem to contradict this:

Note that conversion between NumPy integers is unaffected, so that np.array(-1).astype(np.uint8) 
continues to work and use C integer overflow logic.

Maybe this document should be updated or clarified?

@seberg
Copy link
Member

seberg commented Jan 6, 2023

You are looking at the wrong part of the release note, it talks about Python object conversion/integer conversion. There is a chunk about the new warning here: https://numpy.org/devdocs/release/1.24.0-notes.html#numpy-now-gives-floating-point-errors-in-casts

If it is very confusing, maybe you have an improvement suggestion?

The tip I can give is that the casts between integers are (at least in practice) well defined:

float_arr.astype(np.int64, copy=False).view(np.uint64)
# or
float_arr.astype(np.int64, copy=False).astype(np.uint32)

will work as expected so long the first cast has all numbers within range. That is also the reason why you get sometimes correct "overflow" behavior: The C compiler is translating it to such a cast effectively.

@cmutel
Copy link
Author

cmutel commented Jan 6, 2023

You are of course correct that my issue is related to conversion of floats, not ints. Thanks a lot for the clarification, closing as this is undefined behaviour.

@cmutel cmutel closed this as completed Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants