-
Notifications
You must be signed in to change notification settings - Fork 6
update ldexp
types
#73
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
Conversation
8395aab
to
0b9eeee
Compare
Since this branch is currently being used to debug numpy test failures, I wonder if it'd be worth making this change in this PR: diff --git a/CMakeLists.txt b/CMakeLists.txt
index af4e480..c421df7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -45,6 +45,7 @@ if(WIN32)
string(CONCAT PRECISION_FLAGS
"/fp:fast=2 "
"/Qimf-precision=high "
+ "/Qprec-sqrt "
"/Qprotect-parens "
)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /Ox ${WARNING_FLAGS} ${SDL_FLAGS} ${PRECISION_FLAGS}")
@@ -81,6 +82,7 @@ elseif(UNIX)
"${SDL_FLAGS}"
)
string(CONCAT PRECISION_FLAGS
+ "-prec-sqrt "
"-fprotect-parens "
"-fimf-precision=high "
"-fp-model fast=2 "
Using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within the context of fixing the ldexp
issue, I think this PR is good!
@AndresGuzman-Ballen What's interesting is that it is apparently no longer going to be supported—but if this means test failures, do we know what alternative they recommend? Note that following their recommendation to use |
@ndgrigorian |
@vtavana correct! @ndgrigorian sorry for the confusion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the changelog needs to be updated with that change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
By the way, will we need to increment the patch version number for mkl_umath after this is merged? |
Do you mean the package version? I figured we would cut a new version around the time of the release, but if numpy builds need it, I can go ahead and do it whenever. |
@ndgrigorian good point! I'll keep an eye out for new versions around release time then :) thanks! |
With stock NumPy 2.2.5 on Windows we have
While for Stock NumPy 2.2.5 on Linux we have
The difference is that we have
'q'
on Windows instead of'l'
on Linux.With Intel/Stock NumPy 1.26.4, on both Windows and Linux we have
'l'
.mkl_umath
currently has functions for type 'fl->f' and 'dl->d' which have no equivalent in Stock NumPy 2.2.5 on Windows.This PR resolves this issue. The changes I made is based on what is done in numpy-2.x.x.
With this branch