-
Notifications
You must be signed in to change notification settings - Fork 63
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
update to pybind 2.10.1 #468
Conversation
Tests may not pass till this is merged, this is the same error referenced above: [100%] Linking CXX executable test_bmi_python
terminate called after throwing an instance of 'pybind11::type_error'
what(): Object of type 'type' is not an instance of 'module_'
CMake Error at /usr/local/share/cmake-3.24/Modules/GoogleTestAddTests.cmake:112 (message):
Error running test executable.
Path: '/home/runner/work/ngen/ngen/cmake_build/test/test_bmi_python'
Result: Subprocess aborted
Output: |
Yeah, this does not make sense... if this fixes that problem then the code run in the test should pass. Is this a cache issue again? Is the pybind11 submodule cached? |
@hellkite500 I have a feeling there may be other places that need the And/or... I noticed in the test output that it's failing to build a wheel for bmipy ... after that it says successfully installed, so it may have installed without building a wheel, but I'm not sure if this is new... maybe try installing
|
8731c85
to
b7ae8fc
Compare
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.
This whole thing seems odd because module_ is a thing and this should work... keep in mind for later, because we may very well be losing something by dropping the fact that these object
s are module_
s.
If you look at the usage of any of the |
To support python 3.11, as well as gather some important bug fixes, pybind11 needs to be updated. At the time of this PR 2.10.1 is the latest stable release.
A note on the changes in
InterperterUtils.hpp
, with this version of pybind, themodule_
declarations had to be modified. ngen will compile just fine with the old definitions, but at runtime, you end up with this type of error:But updating them to generic
py::object
types seemed to work, and I tested this change on the existing 2.6.1 pybind as well and there were no issues.I was curious about the members in
Routing_Py_Adapter.hpp
, since it storespy::module_
handles as well, but it seems like they weren't affected (I ran routing simulations with no runtime errors...).Thanks to nmslib/nmslib#488 and microsoft/spacy-ann-linker#7 for pointing the way!
Changes
py::module_
types topy::object
typesTesting
Notes
Checklist
Target Environment support