Skip to content

Conversation

vtavana
Copy link
Contributor

@vtavana vtavana commented May 7, 2025

resolves IntelPython/mkl_fft#165

Main branch

import numpy as np, mkl_umath n = 2**31 data = np.ones(n) mkl_umath.absolute(data) # Intel oneMKL ERROR: Parameter 1 was incorrect on entry to vdAbs. # array([0., 0., 0., ..., 0., 0., 0.], shape=(2147483648,))

This branch

import numpy as np, mkl_umath n = 2**31 data = np.ones(n) mkl_umath.absolute(data) # array([1., 1., 1., ..., 1., 1., 1.], shape=(2147483648,))
@vtavana vtavana self-assigned this May 7, 2025
@vtavana vtavana force-pushed the add-ilp64 branch 8 times, most recently from 69cb600 to 1bfa605 Compare May 8, 2025 00:59
@vtavana vtavana marked this pull request as ready for review May 8, 2025 02:00
@ndgrigorian
Copy link
Collaborator

In general I don't see a problem with this change, but changing a dependency is a pretty big tweak.

@ekomarova @AndresGuzman-Ballen is there any way Intel NumPy tests can be run with this branch to make sure it doesn't introduce any major failures?

@AndresGuzman-Ballen
Copy link
Contributor

@ndgrigorian I suppose I could point mkl_umath's commit in numpy-2.2.5 branch to this branch, but I'd prefer doing that after I can get all subpackages to build and pass tests in our CI

@ekomarova
Copy link
Collaborator

ekomarova commented May 22, 2025

This PR does not break testing with Intel Numpy 2.2.5. All checks are green. I will also check v1.26.4

@ekomarova
Copy link
Collaborator

@ndgrigorian I see some problems in build/tests in numpy 1.26.4. Could you please take a look at it here https://github.com/intel-innersource/libraries.python.intel.condarecipes.numpy-recipe/pull/152?

@vtavana
Copy link
Contributor Author

vtavana commented May 22, 2025

@ndgrigorian I see some problems in build/tests in numpy 1.26.4. Could you please take a look at it here intel-innersource/libraries.python.intel.condarecipes.numpy-recipe#152?

@ekomarova To fix build failure you need to pin setuptools >= 77

@vtavana
Copy link
Contributor Author

vtavana commented May 22, 2025

Linux test failure is because of tolerance, I think it should be patched to fix.

FAILED core/tests/test_umath.py::TestPower::test_power_float - AssertionError: Arrays are not almost equal to 7 decimals unary offset=(0, 0), size=8, dtype=<class 'numpy.float32'>, out of place Mismatched elements: 1 / 8 (12.5%) Max absolute difference: 2.3841858e-07 Max relative difference: 9.7333974e-08 x: array([0. , 0.9999999, 1.4142134, 1.7320508, 1.9999999, 2.236068 , 2.4494896, 2.6457512], dtype=float32) y: array([0. , 1. , 1.4142135, 1.7320508, 2. , 2.236068 , 2.4494898, 2.6457512], dtype=float32) FAILED core/tests/test_umath.py::TestAVXUfuncs::test_avx_based_ufunc - AssertionError: Arrays are not equal Mismatched elements: 1 / 2 (50%) Max absolute difference: 4.7683716e-07 Max relative difference: 6.150229e-08 x: array([2.410054, 7.75316 ], dtype=float32) y: array([2.410054, 7.75316 ], dtype=float32)
@AndresGuzman-Ballen
Copy link
Contributor

@AndresGuzman-Ballen
Copy link
Contributor

@ekomarova in case you're curious how I came to that conclusion, this is some context: #73

@vtavana
Copy link
Contributor Author

vtavana commented May 22, 2025

Windows failure are not related to this PR, they are related to #73
Does numpy-1.26.4 works with main branch of mkl_umath? or we had not tested that recently?

@AndresGuzman-Ballen
Copy link
Contributor

I think numpy 1.26.4 wasn't tested recently with recent mkl_umath changes. I'll push some tweaks so that way the numpy 1.26.4 PR works.

@AndresGuzman-Ballen
Copy link
Contributor

@vtavana how strange...I saw a Windows failure for the ldexp function here when the changes from here seem to be included. Is the ilp64 PR introducing this issue again?

@vtavana
Copy link
Contributor Author

vtavana commented May 22, 2025

@vtavana how strange...I saw a Windows failure for the ldexp function here when the changes from here seem to be included. Is the ilp64 PR introducing this issue again?

I believe there was a change in #73 that is not needed and should be reverted. I created a new PR to revert it #77

@vtavana
Copy link
Contributor Author

vtavana commented Jun 4, 2025

Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

no further concerns from my side, LGTM

@vtavana
Copy link
Contributor Author

vtavana commented Jun 6, 2025

@napetrov Any other suggestion/concern on this PR before merging it?

@napetrov
Copy link
Contributor

napetrov commented Jun 6, 2025

@napetrov Any other suggestion/concern on this PR before merging it?

thanks, look good!

@vtavana vtavana merged commit aafc0cc into main Jun 6, 2025
32 checks passed
@vtavana vtavana deleted the add-ilp64 branch June 6, 2025 15:04
@jankrecke
Copy link

Thank you for the fix! 🙏

Can you comment on when this will be released? 👀

@ekomarova
Copy link
Collaborator

@jankrecke Hi! This should be released soon along with the 2025.2 release, I can't give exact dates, but most likely within a couple of weeks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants