-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Future of npymath library #20880
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
Comments
Note that whether or not this is a priority is kinda coupled with the We could do something proactively, but the urgency is a lot lower as long as there's no need to upgrade from |
my suggestion (Windows only) is as follows:
This approach has the following benefits:
For a test I have successfully build There is a disadvantage as well: One has to consider the situation that someone installs an Numpy version without theses additional DLLs and then Scipy which needs them. Mitigation could be:
|
Not a terrible idea in principle, but it would probably be as much work as shipping a DLL and permanently increase build times for libraries using that C code.
This is probably best. I agree with @carlkl's suggestion's mostly, with two tweaks:
This would be the better option - if you need a newer NumPy, then bump the minimum version. |
I came across this again when building NumPy from source when a wheel was not available. It would be nice to solve this for 1.23 since moving forward more people will be hitting this when they compile NumPy and Scipy from source with a modern visual studio. One complication is that on windows, |
I am becoming convinced that the DLLs directory in windows is not a good target. It is not added to the dll search path (see os.add_dll_directory), rather it is added to |
How exactly? This issue was opened when we discovered it with the mingw-w64 compilers. What's your exact config here, and how does this turn up when building numpy only rather than linking
I'm not sure what |
See this to build Scipy on PyPy using meson: it builds NumPy from source on the CI machine.
Linking is the easy part. The harder problem is to find the DLL at runtime. Like LD_LIBRARY_PATH on Linux, windows has an internal list of directories it will search. For the NumPy/SciPy OpenBLAS dll, we call numpy/numpy/distutils/misc_util.py Lines 2354 to 2359 in 46ec71f
|
I would like to return to the idea of shipping the C sources. Perhaps that would just be as simple as copying the generated I think that would be easy to maintain - in practice, surely easier than working out a general mechanism for Numpy to export a DLL directory? Or do we need that for some other reason? |
I would propose to copy npymath.dll from numpy into scipy's |
Another proposal: As npymath.dll is quite small (I tried this out it in januarý and got a 56kB sized npymath.dll) it could be a good idea to simply load this DLL during numpy import. Once the DLL is loaded into process space it will be available for all packages depending on this DLL. Symbols of this DLL does not interfere with the static linked symbols used by numpy, that means numpy will just ignore this DLL. |
Looking at the code to build I think I like the DLL plan a little better.
Of the three ways of using a DLL, this is probably the nicest one - assuming we won't run into trouble with changes in the library & versioning. |
I agree - that's an excellent idea - preloading the DLL. Would this be the right approach for npyrandom? Or is that more likely to be compiled into projects that don't import Numpy? |
I can't find many users, but SciPy uses both, and Erotemic/xdev@77a5712#diff-ffb8093dfe715993aed3918bfa9b1fb2e59c13d4a5699aab5088df611f960209R230 is an example of use in a random project I hadn't seen before (still uses |
The problem with injecting npymath.dll via I do wonder how far we can go to make npymath more of a header-file-only library, along the lines of eigen. The code in npy_math_internal.h.src](https://github.com/numpy/numpy/tree/main/numpy/core/src/npymath), could be shipped as a header file if we always use the |
Most trigonometric symbols (also the complex ones) are backed by |
Also ping @serge-sans-paille since I am worried this affects gh-21487. One thing I am unclear about is whether using a dynamically linked library here might be problematic for some users because it prevents inlining? A lot of the functions are headers though, so maybe Matti's thought about just making it header-only makes sense? Since |
as described in #20880 (comment), supplying static as well dynamic libraries for npymath and npyrandom could be a solution. |
To make the link bidirectional: conda-forge is currently discussing moving to vc142 on windows (leaning towards implementation soon), perhaps some people here are interested or have an opinion: conda-forge/conda-forge.github.io#1732 |
As it is trivial to build DLLs from the npymath.lib and the npyrandom.lib libraries I propose to use DLLs rather than the VC lib files for the scipy building process on windows with mingw-w64. It is even possible to build one single DLL for the sake of simplicity, which include the object code from both libraries. This procedure was necessary for the windows 32-bit scipy-meson test build anyway. The advantage of this procedure: it works regardless of the VC version. |
I'm finding myself toying with the idea of just shipping the sources again. It's raining issues with
In the end it's not that bad; a lot of that is weird handling of the math library. For SciPy we can probably just ignore all that. It would also help with solving the problem now - we look for the install source files, and if they're missing then we use a vendored copy. With a DLL, that doesn't work. |
Unfortunately one also needs then source to numpy random as well, as numpy.random also depends on npymath symbols. Another idea would be to build numpy with mingw-w64 as well and use that to build scipy. A numpy build of this kind should follow the specifications of CPython (long double == double) and should use the windows umbrella libraries for the math symbols. Later on a scipy wheel build this way should work together with a standard PYPI MSVC build numpy without any problems IMHO. In the end it should be sufficient to exclusively build the minimum numpy version for a given scipy version. |
In the last few weeks I've been working on #24085 that removes the complex structs and uses native complex types instead. In terms of new APIs, this PR adds six new functions that can be used to set the real or imaginary part of a complex number ( I'm now done with all of the changes required on the numpy side and I'm trying to port SciPy to the new version. I'm mostly done with the port, which works okay with #24085, but does not work with older numpy versions and I have a couple of questions:
#ifdef NPY_VERSION < 0x02000000
#define SETRE(x, r) x->real = r
#define SETIM(x, i) x->imag = i
#else
#define SETRE(x, r) npy_csetreal(x, r)
#define SETIM(x, i) npy_csetimag(x, i)
#endif
|
If the only thing needed is those 8 lines, I think we can copy that code into |
We semi do that: If you didn't still build with NumPy 1.x, you could in principle use set What we could do, is put a |
Edited... FYI, here's a data point from a downstream project. I can't simply replace the complex Edit: I did some nasty casting and eliminated the use of Edit 2: In @ngoldbaum pointed out the issue of the half float type above. |
In the last few weeks I've put some time into moving
This setup allows us to both work on In my view, the plan ahead should be the following:
Do people think this all makes sense? Anything else you'd like to see as part of this effort? |
Thanks for the update @lysnikolaou. We briefly touched on this in the community meeting too, and folks seemed to think that indeed there is no issue in making npymath C-only again. If the C++ wrapper is needed, it can live outside of npymath. A few other thoughts:
I don't know about this, but if it has to be moved, 2.0 would be a good time to do it. |
Does anyone know why Is there a reason why it can't live in the NumPy C API? I don't see a problem with it and to me it makes sense that all the C API needed to work with numpy dtypes are in the standard library or provided by numpy. We also recently exposed some functions for working with datetimes, for what that's worth as an argument here. |
Would be great to update the docs on using/extending np.random from compiled code, which requires linking to npyrandom and npymath: https://numpy.org/doc/stable/reference/random/extending.html |
The design of exposing the random number distribution functions only via a static library is a bit unfortunate. In scipy/scipy#19638 (comment) I posted a summary of what SciPy needs from What's exposed in The Numba example already relies on using the actual C sources rather than the static library we ship: https://numpy.org/doc/stable/reference/random/examples/numba_cffi.html. I'd suggest that we change If we do that we don't have to stop shipping the static library and |
That all sounds reasonable, but I never really had a firm first-hand grasp on the problems these static libraries were meant to solve. |
xref gh-25390, which attempts to perform the libnpyrandom surgery. |
So a status update here: I've opened two PRs that try to do a quite big overhaul in the hopes of improving the situation:
I created the libnpymath repo under my own username for the time being and it probably needs to be reviewed, since I changed quite a few things (including a rewrite of the C++ parts to C 🎉 ) So the current state is this:
That means that we can effectively stop shipping both of the static libraries and SciPy will work okay. Should we? Also, if people think all this looks okay, should we move the |
Thanks for the update @lysnikolaou, this is starting to shape up nicely.
Would it make sense to separate out that C++ to C rewrite and get that merged as a separate PR to the main numpy repo first? That would make review easier, because it's then split into smaller chunks and also because the primary reviewers for C++/C and build&packaging topics tend to no be the same.
We should finish the SciPy changes first, and get those in before branching 1.13.x in 1-2 months. And then announce the plan to shop shipping static libraries - which may be on the late side for 2.0, not sure about that one yet. If it isn't feasible for 2.0, then it'll be by 2.2.0 probably if there aren't too many objections.
+1 to that. It also seems like a reasonable name to continue using. Did the symbol naming issue to avoid |
Sure! Actually, I opened a PR for that on my fork as well, cause I figured it might be needed, so that can be reverted, reviewed and reapplied as well, if adding it on numpy first seems like more effort than needed.
The SciPy PR could be merged as-is as far as I'm concerned (after the move of the
Yes, all the internal symbols were renamed to |
I checked the symbol names in the subproject on your SciPy PR with |
the npymath objects are linked statically, so I can't see a problem here. Any problem with symbol names should show up during the linking step. |
Indeed I have tested this and it all plays well. I built a wheel with NumPy 2.0 and then run the whole suite with the wheel alongside both NumPy 2.0 and 1.26. All seems to be fine. My concern is more about the readability of the whole thing.
And then use the Obviously, these can also be post-release fixes/improvements if we're in a hurry to make it before NumPy 2. |
@lysnikolaou where does this work stand? The dust has settled around NumPy 2.0, can we move forward with the separate repo for npmath? |
Proposed new feature or change:
This is an issue to discuss the future of the
npymath
library in Numpy.Numpy ships with a static library in
core/lib
namednpymath.lib
(Windows) orlibnpymath.a
(Unices).It provides a suitable platform-specific implementation of various math routines.
Scipy links to
npymath
during its build process, in various places.We (Scipy) have run into problems linking against Npymath, because of the combination of Numpy's use of the latest Visual Studio toolchain, and Scipy's use of the Mingw-w64 toolchain - discussed in MacPython/numpy-wheels#145 .
The issue that arises is that it is in general more difficult to link static libraries across toolchains, than it is to link against dynamic libraries (e.g. DLLs) or to recompile the sources. This issue is to discuss whether we should think of other solutions. Possible options are:
This issue is to house the discussion of different options.
The text was updated successfully, but these errors were encountered: