-
Notifications
You must be signed in to change notification settings - Fork 39
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
Draft: Follow-up numpy 2.0 (DLLAPI) #451
Conversation
I think I will try to use |
I changed my mind. Since |
For now, something is broken, and it is not clear to me why. I will ask on numpy repo: ImportError: /home/runner/.local/lib/python3.10/site-packages/jiminy_py/core/core.cpython-310-x86_64-linux-gnu.so: undefined symbol: EIGENPY_ARRAY_APIPyArray_RUNTIME_VERSION here is the corresponding issue on Numpy. |
After a long discussion with numpy team, it turns out that numpy API should not be exposed the way it is to avoid compatibility issues. Actually every import of numpy headers should be moved to the source file. It is the only way to ensure that EigenPy public API does not leak numpy internal symbols. |
Maybe moving the discussion here (also). I am still happy to mitigate the linking issue in NumPy if you/ In a sense, the obvious solution I (and others) on NumPy see right now is to ask users to do a normal NumPy include with the typical NumPy pattern:
in most files, and:
in the main module file before any import The question is whether that seems easy enough for the |
@duburcqa Any feedback on this? |
@jcarpent No. Here is the current state of the discussion numpy/numpy#26091. A feedback from Eigenpy team was requested to decide in which direction to go next, since different options are worth to consider. I stopped tracking this issue at that time. |
Apparently, changes were merged into Numpy recently numpy/numpy#26103. It is now possible to expose Numpy symbols in release 2.0 as it used to be previously. I will have a look and try to take advantage of this feature to fix the issue with eigenpy if this solution is OK for you. It is the one involving the minimal amount of changes but the maintainers of Numpy were suggesting it may be worth refactoring eigenpy to avoid exposing Numpy symbols completely. |
@duburcqa I don't think we backported these to 2.0. We could make a (limited) backport to 2.0 if that is necessary for you, though. EDIT: To be clear, not exposing NumPy symbols completely may be a bit harder (although it might make sense to at least consolidate the windows/linux divergence in either direction). Downstream can work around this already (at least I think) by properly including NumPy first. The thing to do here might only be to enforce or at least note that. |
@seberg Thank you for the clarification. Indeed I made the assumption of backport in Numpy 2.0 since this release was still in a release candidate and not definitive, but it turns out I was wrong ! |
@duburcqa Could you try the current devel of EigenPy? |
@jcarpent the issue isn't that it doesn't work at all with NumPy 2. What fails due to the current setup is that if you compile for NumPy 2 that extension (which uses eigenpy) cannot use an |
Exactly, @seberg summed it up pretty well. |
"I'm landing", thanks for the clear clarification. |
Thank for digging the issue. We can close this PR since #496 had been merged. We know this will not allow binary compatibility but cleaning the way we expose NumPy API will be really hard, especially because eigenpy use a lot of template. If it become an issue for some users we will try to address this but this will create a new major release I think. |
No description provided.