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

Add numpy 2 support #434

Merged
merged 11 commits into from
Oct 17, 2024
Merged

Add numpy 2 support #434

merged 11 commits into from
Oct 17, 2024

Conversation

jparismorgan
Copy link
Contributor

@jparismorgan jparismorgan commented Jul 3, 2024

What

Updates to support NumPy 2. NumPy 2.0 as a major release changes the C ABI, so any package that builds against the NumPy C API like shapely will have to be rebuilt with numpy 2.0 to be able to run with numpy 2.0.

Specifically:

  • We require that packages are built with numpy>=2.0.0
  • We require that at runtime numpy>=1.25.0 is present

See:

Testing

  • Existing tests pass.

TODO

In a future PR we could follow this advice and test against the earliest numpy version which we support:

Screenshot 2024-07-12 at 4 08 34 PM

Opened SC-50896 to track this.

Comment on lines +404 to +405
size = np.int64(schema.domain.dim(1).domain[1]) + 1
dimensions = np.int64(schema.domain.dim(0).domain[1]) + 1
Copy link
Contributor Author

@jparismorgan jparismorgan Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We make this change b/c of changes to numpy data type promotion.

With numpy 1 and the original code we were getting:

[ingestion@read_source_metadata] schema.domain.dim(1).domain[1]: 2147483647 <class 'numpy.int32'>
[ingestion@read_source_metadata] size: 2147483648 <class 'numpy.int64'>
[ingestion@read_source_metadata] dimensions: 3 <class 'numpy.int64'>

With numpy 2 and the original code we instead were getting:

[ingestion@read_source_metadata] schema.domain.dim(1).domain[1]: 2147483647 <class 'numpy.int32'>
[ingestion@read_source_metadata] size: -2147483648 <class 'numpy.int32'>
[ingestion@read_source_metadata] dimensions: 3 <class 'numpy.int32'>

This is because in size = schema.domain.dim(1).domain[1] + 1 the + 1 used to cause a cast to int64 (which is required because 2147483647 is int32 max).

As mentioned in https://numpy.org/devdocs/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion, numpy no longer casts automatically:

Screenshot 2024-07-12 at 3 55 46 PM

So here we explicitly cast to int64 so that we return the same value as we did before.

@@ -2016,7 +2016,7 @@ def consolidate_partition_udf(
prev_index = partial_indexes[0]
i = 0
for partial_index in partial_indexes[1:]:
s = slice(int(prev_index), int(partial_index - 1))
s = slice(int(prev_index), int(partial_index) - 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is also because numpy 2 does not do data type promotion.

With numpy 1 and the original code we were getting:

[ingestion@consolidate_partition_udf] partial_indexes [  0   0   1   1   5   6  14  14  14  18  23  28  42  44  44  44  50  55
  58  60  61  61  61  66  66  81  81  84  84  86  91  94 101 108 110 114
 118 125 126 127 127 127 135 135 141 142 143 143 149 154 157 161 178 191
 200 201 209 214 214 230 233 236 240 242 243 248 257 257 275 276 278 282
 283 290 290 291 298 316 324 324 332 335 335 343 343 347 350 353 356 373
 374 379 382 391 391 391 398 399 405 412 421] <class 'numpy.ndarray'> uint64
[ingestion@consolidate_partition_udf] prev_index 0 <class 'numpy.uint64'>
[ingestion@consolidate_partition_udf] partial_index 0 <class 'numpy.uint64'>
[ingestion@consolidate_partition_udf] s slice(0, -1, None) <class 'slice'>

With numpy 2 and the original code we were getting:

[ingestion@consolidate_partition_udf] partial_indexes [  0   0   1   1   5   6  14  14  14  18  23  28  42  44  44  44  50  55
  58  60  61  61  61  66  66  81  81  84  84  86  91  94 101 108 110 114
 118 125 126 127 127 127 135 135 141 142 143 143 149 154 157 161 178 191
 200 201 209 214 214 230 233 236 240 242 243 248 257 257 275 276 278 282
 283 290 290 291 298 316 324 324 332 335 335 343 343 347 350 353 356 373
 374 379 382 391 391 391 398 399 405 412 421] <class 'numpy.ndarray'> uint64
[ingestion@consolidate_partition_udf] prev_index 0 <class 'numpy.uint64'>
[ingestion@consolidate_partition_udf] partial_index 0 <class 'numpy.uint64'>
[ingestion@consolidate_partition_udf] s slice(0, 18446744073709551615, None) <class 'slice'>

Notice that we get slice(0, 18446744073709551615, None) instead of slice(0, -1, None). To fix this we can cast before subtracting, which we do here.

@jparismorgan jparismorgan marked this pull request as ready for review October 16, 2024 21:42
Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jparismorgan jparismorgan merged commit bab5ada into main Oct 17, 2024
7 checks passed
@jparismorgan jparismorgan deleted the jparismorgan/numpy2 branch October 17, 2024 16:01
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.

4 participants