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

Support NumPy 2.0 #550

Merged
merged 6 commits into from
Jul 1, 2024
Merged

Support NumPy 2.0 #550

merged 6 commits into from
Jul 1, 2024

Conversation

mhostetter
Copy link
Owner

@mhostetter mhostetter commented Jun 22, 2024

Probably should merge into release/0.4.x.

@mhostetter mhostetter added numpy compatibility Affects compatibility with NumPy numba compatibility Affects compatibility with Numba labels Jun 22, 2024
@mhostetter mhostetter force-pushed the numpy-2.0 branch 3 times, most recently from 81fdd1f to 4c9f6b5 Compare June 22, 2024 18:41
@iyanmv
Copy link
Contributor

iyanmv commented Jul 1, 2024

The trick of converting -1 to the max of the unsigned dtype does not work with NumPy 2.0 anymore. I think this should be enough to fix it:

diff --git a/src/galois/_polys/_dense.py b/src/galois/_polys/_dense.py
index b1b3e28..4f45d78 100644
--- a/src/galois/_polys/_dense.py
+++ b/src/galois/_polys/_dense.py
@@ -487,7 +487,7 @@ class roots_jit(Function):
         # Test if 0 is a root
         if nonzero_degrees[-1] != 0:
             roots.append(0)
-            powers.append(-1)
+            powers.append(np.iinfo(nonzero_coeffs.dtype).max)
 
         # Test if 1 is a root
         _sum = 0

With this patch in addition to your 3 commits all tests pass again with NumPy 2.0 🙂

@iyanmv
Copy link
Contributor

iyanmv commented Jul 1, 2024

Sorry, I forgot to include in the diff one more needed change to pass all tests. Here it is:

diff --git a/src/galois/_fields/_array.py b/src/galois/_fields/_array.py
index 5cf847d..2ff32fa 100644
--- a/src/galois/_fields/_array.py
+++ b/src/galois/_fields/_array.py
@@ -988,7 +988,7 @@ class FieldArray(Array, metaclass=FieldArrayMeta):
         if x.ndim == 0:
             order = 1 if x == 0 else field.characteristic
         else:
-            order = field.characteristic * np.ones(x.shape, dtype=np.int64)
+            order = field.characteristic * np.ones(x.shape, dtype=x.dtype)
             order[np.where(x == 0)] = 1
 
         return order

@iyanmv
Copy link
Contributor

iyanmv commented Jul 1, 2024

If it's easier for you, I have made both changes available in this branch. I guess you can cherry-pick those commits directly here:
numpy-2.0...iyanmv:galois:numpy-2.0

@mhostetter mhostetter changed the base branch from release/0.3.x to release/0.4.x July 1, 2024 22:15
@mhostetter mhostetter merged commit 4921897 into release/0.4.x Jul 1, 2024
49 checks passed
@mhostetter
Copy link
Owner Author

Thanks for the fixes @iyanmv. I modified them slightly (found a bug in my code in one, made your solution more general in the other). Can you test release/0.4.x and see if all is good there? If so, I plan to release 0.4.0 with NumPy 2.0 support.

@mhostetter mhostetter deleted the numpy-2.0 branch July 1, 2024 23:40
@iyanmv
Copy link
Contributor

iyanmv commented Jul 2, 2024

That branch works well on Arch Linux 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
numba compatibility Affects compatibility with Numba numpy compatibility Affects compatibility with NumPy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants