Skip to content
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

pypy3.9 error related to pybind11::error_already_set, unexpected indent #25

Closed
ngam opened this issue Apr 11, 2022 · 25 comments
Closed

Comments

@ngam
Copy link

ngam commented Apr 11, 2022

I am running into this error with pypy3.9 only (here). I've seen it elsewhere as well, e.g. scipy.optimize. So maybe this may become widespread as the migration is underway? Before debugging any further, I wanted to bring it to your attention asap.

Any thoughts @isuruf + @mattip?

terminate called after throwing an instance of 'pybind11::error_already_set'
  what():  IndentationError: ('unexpected indent', ('<string>', 2, 1, '        class pybind11_static_property(property):\n', 2))
@mattip
Copy link
Contributor

mattip commented Apr 11, 2022

That has been solved on pybind11 HEAD by @henryiii and is waiting for a pybind11 release, or a patch to the feedstock.

@henryiii
Copy link

I thought this was backported and released in 2.9.2? Most users pin pybind11 exactly rather than using the feedstock, which is why this tends to break (see iminuit, boost-histogram, and awkward, all of which need pybind11 bumps to work with PyPy 3.9).

@henryiii
Copy link

Is there a way to do a run constrained to 2.9.2+ here, though? That might be useful.

@ngam
Copy link
Author

ngam commented Apr 11, 2022

I ran this with pybind11 2.9.2 (under test: requires:) and the error persisted. Where else would one add pybind11?

@henryiii
Copy link

Via submodule, then including the files in SDist. (Or any way of including the files in the SDist, it's header-only). pybind11 is (only) a build-time requirement, not a test requirement. This is by far the most common way people include pybind11.

@henryiii
Copy link

See the fix in conda-forge/iminuit-feedstock#76 for example.

@henryiii
Copy link

TL;DR: if it's not already in the conda-forge requirements, adding it will have no effect, it's being pulled in some other way (usually included in the SDist).

@ngam
Copy link
Author

ngam commented Apr 11, 2022

Ah I see. Thanks, I will add a ppatch to force the recipe to fetch 2.9.2

@ngam
Copy link
Author

ngam commented Apr 11, 2022

Closing but this will likely resurface in quite a few packages...

@ngam ngam closed this as completed Apr 11, 2022
@henryiii
Copy link

Yes, that's why I think a runtime constraint would at least be a minor help. We should probably push toward using the conda-forge package more often instead of burning in a locked pybind11 version (for our Scikit-HEP packages, CC @HDembinski and @jpivarski and myself). This also enables ABI migrations which were added for the pybind11 recipe a month or two ago. But that's likely not going to happen everywhere overnight, so this will probably continue to be a problem for a while.

@henryiii
Copy link

@isuruf can we do things like run_constrained on a meta feedstock like this?

@ngam
Copy link
Author

ngam commented Apr 11, 2022

Reopening because pybind11 (via direct fetch in cmakelists) results in ppc failure:

ImportError: /home/conda/feedstock_root/build_artifacts/dm-tree_1649707734634/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol/lib/python3.7/site-packages/tree/_tree.cpython-37m-powerpc64le-linux-gnu.so: undefined symbol: _ZN4absl12lts_2021032420raw_logging_internal6RawLogENS0_11LogSeverityEPKciS4_z
[1956]()

@ngam ngam reopened this Apr 11, 2022
@isuruf
Copy link
Member

isuruf commented Apr 11, 2022

@isuruf can we do things like run_constrained on a meta feedstock like this?

If the downstream packages bundle pybind11, there's nothing we can do.

@ngam
Copy link
Author

ngam commented Apr 11, 2022

The problem is that people would likely just fetch pybind11 in cmakelists, etc. and it's pretty difficult to uniformly address that. Imperfect, but probably the best course of action would've been to continue liking the bad indent (as pypy3.7 and pypy3.8 did) which would have necessitated a patch to pypy3.9 itself rather than all downstreams

@ngam
Copy link
Author

ngam commented Apr 11, 2022

@isuruf can we do things like run_constrained on a meta feedstock like this?

If the downstream packages bundle pybind11, there's nothing we can do.

@isuruf, thought on the resulting ppc error?

@henryiii
Copy link

That's in CPython, though, right?

If the downstream packages bundle pybind11, there's nothing we can do.

Yes, but we can at least help out anyone using the feedstock. Though if users are using the feedstock, this would only help if they were limiting the pybind11 version for some reason, otherwise they'd be getting the latest version anyway.

which would have necessitated a patch to pypy3.9 itself rather than all downstreams

This wouldn't fix the official PyPy binaries, so unless upstream (un)fixed this, it would be surprising (packages that worked in conda-forge would not work outside of it). Hopefully upstreams are getting patches updating them to Pybind11 2.9.2, so they will work on PyPy 3.9 regardless of where it comes from. If you manually hardcode a pinned version of something, you should be updating it frequently, otherwise things start breaking like this. (We've put a lot of fixes in since 2.6.2!)

@ngam
Copy link
Author

ngam commented Apr 11, 2022

That's in CPython, though, right?

ppc error? All ppc runs, i.e. CPy and PyPy, as a result of bumping pybind11 to 2.9.2

@mattip
Copy link
Contributor

mattip commented Apr 11, 2022

I would be happy to do whatever is easiest for projects.

@ngam
Copy link
Author

ngam commented Apr 11, 2022

I think it is too late to really change things --- we just need to help people overcome this error by providing the newest pybind11. (I have no idea what went wrong with ppc, so that may end up being a separate issue specific to pybind11, not pypy)

@henryiii
Copy link

henryiii commented Apr 11, 2022

Personally okay with forcing people to upgrade pybind11, but I'm biased. :D There are other fixes there for PyPy 3.9 (including a discovery issue and maybe a segfault that might be related or might not be). So just "fixing" (unfixing) this one thing upstream in PyPy might not really help anyone.

@henryiii
Copy link

We do have a little contributed code supporting ppc64le, and we have users on it (and other feedstocks are using it), but we don't have dedicated testing in pybind11 for ppc64le.

@mattip
Copy link
Contributor

mattip commented Apr 11, 2022

I wanted to point out google-deepmind/tree#82 which is similar to the ppc failure, but @ngam is already working on it 🚀

@ngam
Copy link
Author

ngam commented Apr 11, 2022

I think pybind11 upgrade may have uncovered a specific abseil error... that's where I am following this... will update if I successfully resolve this

@henryiii
Copy link

Abseil was producing warnings about not understanding ppc64le in the logs, FWIW.

@ngam
Copy link
Author

ngam commented Apr 15, 2022

Closing this as the remaining ppc problem is unrelated to pybind11 or pypy3.8/3.9

Feel free to reopen if you deem useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants