You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
BUG: Undefined symbol when compiling shared library for numpy 2.0 #26091
I am getting some undefined symbol error when compiling a shared library (C Python Module) that is linking against yet another shared library (C Python Module), both compiled against Numpy 2.0. Everything was fine before the update of Numpy to 2.0.0b1.
Does anyone have any idea regarding what is causing this issue?
I could write one if really necessary. Basically I compile the library Eigenpy and then I linked my own module on it. It is the latter that contains undefined symbols.
What I don't understand is that PyArray_RUNTIME_VERSION should not be a symbol in the first place no ? By looking at the source code it looks like a preprocessor macro.
The point is that you use Eigens(?) PY_ARRAY_UNIQUE_SYMBOL together with NPY_NO_IMPORT from a different compilation unit and Eigen has not been recompiled with NumPy 2.
If that is a serious use-case I have to think a bit how to deal with it.
If that is a serious use-case I have to think a bit how to deal with it.
It is hard to tell for sure if this is intended or not... But using Eigenpy to expose Eigen for other libraries is definitely a serious usecase. The library is used by pinocchio, hpp-fcl, crocoddyl... and many others INRIA projects.
I think this pattern is coming from Boost::Python which introduces it here. Both Eigenpy and my own library are using Boost::Python over pybind11 to generate bindings for historical reasons.
Yeah, but boost python is header only. While here it seems this isn't a single compilation unit. At least that is my only explanation.
The only way this makes sense to me is that for some reason you are finding an older eigenpy version at runtime. It is not even clear to me that it makes any sense at all for eigenpy's numpy.hpp to be public API at all.
So, I am not clear whether we need a solution that always works, a work-around for these odder cases where it is necessary for some reason, or nothing at all and some part of the project just has not been recompiled with NumPy 2 (e.g. compilation cache).
Well we'll need to find a solution whether in NumPy or not, but it is still unclear where/what that is.
I think that if you just add a n#include "numpy/ndarrayobject.h" before the other includes things will magically work, since you are probably not using the NumPy API (directly) in a way that requires an explicit import (i.e. you use it through a boost::python::numpy but that seems its own compilation unit).
(I.e. you would get a static define for these symbols, and would be expected to call import_array(), but since you don't actually need that API at all, things are fine.)
I still suspect eigenpy shouldn't transitively expose the NumPy API (which it does by setting unique-symbol and including ndarrayobject.h for you). And I also don't think it needs to at all.
But, while I think eigenpy doesn't really need to, I am not sure that no-one else has a better reason.
(I wish I there was a better way to do this unique symbol dance.)
i.e. you use it through a boost::python::numpy but that seems its own compilation unit
Unfortunately, this is a wrong assumption. Both Eigenpy and my own library are using barebone C-API of Numpy because not everything has been exposed in boost::python::numpy, typically when it comes to custom scalar type registration or memory alignment handling.
Both Eigenpy and my own library are using barebone C-API
Let me summarize the situation here a bit:
NumPy is set up to allow a project to have multiple c/cpp files which share one API table with a single import_array() call. This is the "Unique Symbol + No Import" mechanism.
This is really mainly meant for multiple C-files which are bundled into a single .so I think. Not really for sharing across library lines (but nothing prevents that).
eigenpy sets the unique symbol, which means it shares it's API table with any project that includes it (i.e. at runtime you must find the eigenpy.so).
N.B.: The point of the NumPy setup is exactly to avoid any direct runtime symbol lookup, the API is instead shared through Python API.
This means that:
eigenpytransitively exposes NumPy C-API to user (although I doubt they need this, i.e. users could include numpy themselves). However, eigenpy does this even if users do not need any NumPy API.
In your situation, you do need the NumPy API, and you could take care of the ndarrayobject.h and import_array() call yourself in principle. In fact, a (maybe strange) approach would be to:
Add #include "eigenpy/numpy.hpp" before other includes that might pull in NumPy.
Add if (_import_array() < 0) { throw ... } somewhere in the module init (where you call eigenpy::enableEigenPy())
That will define the symbol locally to your library/project, which, while using the same name, should overshadow those from eigenpy, I think.
"Simple" Solution: We can easily redefine PyArray_RUNTIME_VERSION to be one of:
// First could be 0 in some cases, but its fine 0 means "old", second is a function call.
#define PyArray_RUNTIME_VERSION ((int)PyArray_API[1])
#define PyArray_RUNTIME_VERSION PyArray_GetNDArrayCVersion()
both of which should be fine in practice.
Problem:
Due to this transitive export you never call import_array() from your library which means you have no compatibility checks:
If your library is compiled with NumPy 1.x it might not notice it is running in NumPy 2 and just break/return wrong things.
You cannot really use new NumPy API because you never do the runtime check if that API is actually available.
Unfortunately, I guess the NumPy headers make it somewhat easy to do this even unintended.
So we can change the above. Things will work, but you lose those checks and balances. I suspect we can live with it, but...
... What bothers me most, is not what we do right now, but that I would like a way forward so that eigenpy stops transitively exporting the NumPy API, ideally without losing anything.
For example, a pattern of #define DEFINE_EIGENPY_ENABLE which must be added to be able to call eigenpy::enableEigenPy(); and making eigenpy::enableEigenPy a static inline entry-point which calls import_array()/import_numpy() as static inline would work. But in practice, I am not sure if some libraries don't even get away without it entirely, so not sure how well just overshadowing works.
Thank you @seberg , all these details are helping a lot. I will wait for a feedback from eigenpy team before deciding going for one direction or the other.
One new observation: Windows defaults to private symbols for shared libraries. Thus, it seems like your code is only working because you force static linking of eigenpy on windows.
And I guess there is a good chance the only reason you do this is the NumPy API use!?
Windows defaults to private symbols for shared libraries. Thus, it seems like your code is only working because you force static linking of eigenpy on windows.
Indeed I do, but only for historically reason (it was easier at the beginning to ensure that the C module can be loaded on any machine if it has no external dependency at runtime). Now, I'm "repairing" the generated wheel using machomachomangler (much like what auditwheel does on Linux), so it is not an issue anymore. But since it was working this way, I never tried to go back to shared library on Windows. Especially because the scenario I mentioned before only makes sense on Unix, since everybody that is using Eigenpy for serious research and business is using MacOS or Linux but never Windows, so inter-operability is much less revenant in this case.
No, this is the reason for this old PR stack-of-tasks/eigenpy@7a57c19, but that isn't enough at all, since eigenpy makes all of it's internals public and they randomly use the NumPy API. You must link eigenpy statically on windows unless you either:
deal with NumPy includes before eigenpy does so.
Or just happen to not use any API which might need NumPy.
I also do wonder how much you actually care about this behavior: The wheels you package must include eigenpy so presumably you can make sure it is compiled with the newest NumPy. conda-forge, etc. will also be able to just handle those dependencies, fine I presume.
For NumPy, I think we ideally "break" eigenpy here. That is, default to the windows behavior and hide the import away (up to you to undo that). That doesn't make existing usage safer (it could crash in weird ways if not recompiled for NumPy 2).
Now, since compilers optimize things away that are unused, I could imagine things are actually somewhat fine after that, although it would be nice if eigenpy could at least only include ndarraytypes.h rather than (nd)arrayobject.h if imported as public API (and remove the #define Py_ARRAY_UNIQUE_SYMBOL and #define NPY_NO_IMPORT).
I don't think that works for all files, but maybe for all files which get included by the fwd.hpp.
That will mean that someeigenpy features will fail to link/compile. Most likely nobody will notice that part (you and pinoccio seem to only really call a few initialize functions from it.
Of course jiminy-py uses the NumPy API! So it'll have to implement the import_array call and the unique-symbol, etc. which is currently implicitly done by eigenpy. If you add that to jiminy-py it will just work perfectly with any eigenpy version.
It might be nice to simplify the include pattern a bit at least for C++, I can imagine that would be easy (i.e. a bit similar to what pybind11 is doing).
I also wish we had a numpy/stubs.h header so that a project like eigenpy could define its function with struct PyArray_Descr *func(struct PyArrayObject *arr) without including the full ndarraytypes.h header.
Yes, I agree with you. What I meant is that I just never tried to link dynamically because I wanted to link statically from the start to improve portability. I never had in mind to link dynamically. But yes for sure, it would not work anyway.
I am considering escalating this a bit for you (because I think it clarifies things and is generally better), see gh-26103.
I.e. breaking the linking always, unless you add a define. I am not sure that is the right thing, but this whole windows oddity about windows needing static linking anyway makes me feel this is all very brittle.
But I am not sure... I am also fine with just "helping" you here (could be tied into that same define also). Although I am not even sure that the failing use-case is important (dynamically linking with an eigenpy that was compiled with an older NumPy version than your own library).
I am not sure that there is anything actionable left here? The situation is a bit unfortunate, but I to some degree asking users to include numpy first seems right for such a broad re-export of API.
@seberg I wanted to give you a quick update regarding this issue.
In the end, I decided to switch to defining my own unique symbol via Py_ARRAY_UNIQUE_SYMBOL as you suggested (implementing import_array and so on). As expected, everything is working perfectly.
I'm afraid that this whole issue, ultimately leading to your team offering a way to revert the new behaviour via NPY_API_SYMBOL_ATTRIBUTE, was caused by a poor understanding of how Numpy C API works internally (at least on my side). I think this point is pretty clear for me now, and I agree with you that sharing Numpy symbols across libraries beyond the boundary of each compile unit should be avoided, since it is irrelevant in the vast majority of cases if not all and fragile overall (requiring static linking on Windows for instance).
Activity
charris commentedon Mar 20, 2024
What update?
duburcqa commentedon Mar 20, 2024
Before the migration to Numpy 2.0.0b1.
ngoldbaum commentedon Mar 20, 2024
duburcqa commentedon Mar 20, 2024
Yes, that is what I was afraid of... I was hoping it could be useful for someone involved in the development of numpy 2.0.
I'm building everything myself from source, so yes I'm sure I'm using the exact same numpy version (2.0.0b1).
duburcqa commentedon Mar 20, 2024
What I don't understand is that
PyArray_RUNTIME_VERSIONshould not be a symbol in the first place no ? By looking at the source code it looks like a preprocessor macro.ngoldbaum commentedon Mar 20, 2024
duburcqa commentedon Mar 20, 2024
OK I think the difference is here:
Numpy 2.0.0b1
Numpy 1.26.4
duburcqa commentedon Mar 20, 2024
Here Is what Eigenpy is doing:
Does it make sense to you or is it hard to tell ?
seberg commentedon Mar 20, 2024
The point is that you use Eigens(?)
PY_ARRAY_UNIQUE_SYMBOLtogether withNPY_NO_IMPORTfrom a different compilation unit and Eigen has not been recompiled with NumPy 2.If that is a serious use-case I have to think a bit how to deal with it.
ngoldbaum commentedon Mar 20, 2024
So eigenpy is defining
PY_ARRAY_UNIQUE_SYMBOLto beEIGENPY_ARRAY_API, then it#include's the numpy headers.__multiarray_api.hhas this block:which presumably your build hits.
I'm not sure offhand if we test this sort of build, ping @seberg since he did this work on adding
PyArray_RUNTIME_VERSIONduburcqa commentedon Mar 20, 2024
It is indeed defining
NO_IMPORT_ARRAYhere:Eigenpy has been recompiled with NumPy 2.
It is hard to tell for sure if this is intended or not... But using Eigenpy to expose Eigen for other libraries is definitely a serious usecase. The library is used by pinocchio, hpp-fcl, crocoddyl... and many others INRIA projects.
duburcqa commentedon Mar 20, 2024
I think this pattern is coming from
Boost::Pythonwhich introduces it here. Both Eigenpy and my own library are usingBoost::Pythonoverpybind11to generate bindings for historical reasons.seberg commentedon Mar 20, 2024
Yeah, but boost python is header only. While here it seems this isn't a single compilation unit. At least that is my only explanation.
The only way this makes sense to me is that for some reason you are finding an older
eigenpyversion at runtime. It is not even clear to me that it makes any sense at all foreigenpy'snumpy.hppto be public API at all.So, I am not clear whether we need a solution that always works, a work-around for these odder cases where it is necessary for some reason, or nothing at all and some part of the project just has not been recompiled with NumPy 2 (e.g. compilation cache).
duburcqa commentedon Mar 20, 2024
Indeed ! This is it... I'm explicitly doing this and I forgot about it... Sorry about that.
9 remaining items
seberg commentedon Mar 20, 2024
Well we'll need to find a solution whether in NumPy or not, but it is still unclear where/what that is.
I think that if you just add a n
#include "numpy/ndarrayobject.h"before the other includes things will magically work, since you are probably not using the NumPy API (directly) in a way that requires an explicit import (i.e. you use it through aboost::python::numpybut that seems its own compilation unit).(I.e. you would get a static define for these symbols, and would be expected to call
import_array(), but since you don't actually need that API at all, things are fine.)I still suspect
eigenpyshouldn't transitively expose the NumPy API (which it does by setting unique-symbol and includingndarrayobject.hfor you). And I also don't think it needs to at all.But, while I think eigenpy doesn't really need to, I am not sure that no-one else has a better reason.
(I wish I there was a better way to do this unique symbol dance.)
duburcqa commentedon Mar 20, 2024
Unfortunately, this is a wrong assumption. Both Eigenpy and my own library are using barebone C-API of Numpy because not everything has been exposed in
boost::python::numpy, typically when it comes to custom scalar type registration or memory alignment handling.seberg commentedon Mar 21, 2024
Let me summarize the situation here a bit:
import_array()call. This is the "Unique Symbol + No Import" mechanism..soI think. Not really for sharing across library lines (but nothing prevents that).eigenpysets the unique symbol, which means it shares it's API table with any project that includes it (i.e. at runtime you must find theeigenpy.so).This means that:
eigenpytransitively exposes NumPy C-API to user (although I doubt they need this, i.e. users could include numpy themselves). However,eigenpydoes this even if users do not need any NumPy API.ndarrayobject.handimport_array()call yourself in principle. In fact, a (maybe strange) approach would be to:#include "eigenpy/numpy.hpp"before other includes that might pull in NumPy.if (_import_array() < 0) { throw ... }somewhere in the module init (where you calleigenpy::enableEigenPy())That will define the symbol locally to your library/project, which, while using the same name, should overshadow those from
eigenpy, I think."Simple" Solution: We can easily redefine
PyArray_RUNTIME_VERSIONto be one of:both of which should be fine in practice.
Problem:
import_array()from your library which means you have no compatibility checks:So we can change the above. Things will work, but you lose those checks and balances. I suspect we can live with it, but...
... What bothers me most, is not what we do right now, but that I would like a way forward so that
eigenpystops transitively exporting the NumPy API, ideally without losing anything.For example, a pattern of
#define DEFINE_EIGENPY_ENABLEwhich must be added to be able to calleigenpy::enableEigenPy();and makingeigenpy::enableEigenPya staticinlineentry-point which callsimport_array()/import_numpy()as staticinlinewould work. But in practice, I am not sure if some libraries don't even get away without it entirely, so not sure how well just overshadowing works.mattip commentedon Mar 21, 2024
Maybe we should get some eigenpy people in on this: @jcarpent or @jorisv?
duburcqa commentedon Mar 21, 2024
Thank you @seberg , all these details are helping a lot. I will wait for a feedback from eigenpy team before deciding going for one direction or the other.
seberg commentedon Mar 21, 2024
One new observation: Windows defaults to private symbols for shared libraries. Thus, it seems like your code is only working because you force static linking of
eigenpyon windows.And I guess there is a good chance the only reason you do this is the NumPy API use!?
duburcqa commentedon Mar 21, 2024
Indeed I do, but only for historically reason (it was easier at the beginning to ensure that the C module can be loaded on any machine if it has no external dependency at runtime). Now, I'm "repairing" the generated wheel using
machomachomangler(much like whatauditwheeldoes on Linux), so it is not an issue anymore. But since it was working this way, I never tried to go back to shared library on Windows. Especially because the scenario I mentioned before only makes sense on Unix, since everybody that is using Eigenpy for serious research and business is using MacOS or Linux but never Windows, so inter-operability is much less revenant in this case.seberg commentedon Mar 21, 2024
(sorry for the long post...)
No, this is the reason for this old PR stack-of-tasks/eigenpy@7a57c19, but that isn't enough at all, since
eigenpymakes all of it's internals public and they randomly use the NumPy API. You must linkeigenpystatically on windows unless you either:I also do wonder how much you actually care about this behavior: The wheels you package must include
eigenpyso presumably you can make sure it is compiled with the newest NumPy.conda-forge, etc. will also be able to just handle those dependencies, fine I presume.For NumPy, I think we ideally "break"
eigenpyhere. That is, default to the windows behavior and hide the import away (up to you to undo that). That doesn't make existing usage safer (it could crash in weird ways if not recompiled for NumPy 2).Now, since compilers optimize things away that are unused, I could imagine things are actually somewhat fine after that, although it would be nice if
eigenpycould at least only includendarraytypes.hrather than(nd)arrayobject.hif imported as public API (and remove the#define Py_ARRAY_UNIQUE_SYMBOLand#define NPY_NO_IMPORT).I don't think that works for all files, but maybe for all files which get included by the
fwd.hpp.That will mean that some
eigenpyfeatures will fail to link/compile. Most likely nobody will notice that part (you and pinoccio seem to only really call a few initialize functions from it.Of course
jiminy-pyuses the NumPy API! So it'll have to implement theimport_arraycall and the unique-symbol, etc. which is currently implicitly done by eigenpy.If you add that to
jiminy-pyit will just work perfectly with anyeigenpyversion.It might be nice to simplify the include pattern a bit at least for C++, I can imagine that would be easy (i.e. a bit similar to what
pybind11is doing).I also wish we had a
numpy/stubs.hheader so that a project likeeigenpycould define its function withstruct PyArray_Descr *func(struct PyArrayObject *arr)without including the fullndarraytypes.hheader.duburcqa commentedon Mar 21, 2024
Yes, I agree with you. What I meant is that I just never tried to link dynamically because I wanted to link statically from the start to improve portability. I never had in mind to link dynamically. But yes for sure, it would not work anyway.
seberg commentedon Mar 21, 2024
I am considering escalating this a bit for you (because I think it clarifies things and is generally better), see gh-26103.
I.e. breaking the linking always, unless you add a define. I am not sure that is the right thing, but this whole windows oddity about windows needing static linking anyway makes me feel this is all very brittle.
But I am not sure... I am also fine with just "helping" you here (could be tied into that same define also). Although I am not even sure that the failing use-case is important (dynamically linking with an eigenpy that was compiled with an older NumPy version than your own library).
jorisv commentedon Sep 26, 2024
Hello,
We use the
NPY_API_SYMBOL_ATTRIBUTEdefine you added in #26103 to address our linking issue.Thank for your time and your help.
seberg commentedon Sep 27, 2024
I am not sure that there is anything actionable left here? The situation is a bit unfortunate, but I to some degree asking users to include numpy first seems right for such a broad re-export of API.
duburcqa commentedon Nov 18, 2024
@seberg I wanted to give you a quick update regarding this issue.
In the end, I decided to switch to defining my own unique symbol via
Py_ARRAY_UNIQUE_SYMBOLas you suggested (implementingimport_arrayand so on). As expected, everything is working perfectly.I'm afraid that this whole issue, ultimately leading to your team offering a way to revert the new behaviour via
NPY_API_SYMBOL_ATTRIBUTE, was caused by a poor understanding of how Numpy C API works internally (at least on my side). I think this point is pretty clear for me now, and I agree with you that sharing Numpy symbols across libraries beyond the boundary of each compile unit should be avoided, since it is irrelevant in the vast majority of cases if not all and fragile overall (requiring static linking on Windows for instance).