Skip to content

Conversation

@zjgarvey
Copy link
Collaborator

@zjgarvey zjgarvey commented Oct 20, 2025

I think this is the simplest approach, for now, to resolve #4343

It would be good to eventually finish #4348 ; however, it became a bit too much to rework the generated sources scripts in a timely fashion.

See also another parallel attempt to address the ci problems: #4345

This PR modifies the Cmake pytorch configure function to simply not set any TORCH_CXX_FLAGS whenever pytorch is missing the old PYBIND_BUILD_ABI tag. I think whatever compiler flags we were pushing through to make pybind think we are GCC and to use a specific ABI version is just completely unnecessary now. I was worried we might need to update our pybind version in the requirements, but it appears to not be relevant.

Additionally, nightly pins are updated and small fixes are made to resolve misc failures in tests after the bump.

The old path is still left in case we need to build against older versions of pytorch.

Signed-off-by: zjgarvey <[email protected]>
Signed-off-by: zjgarvey <[email protected]>
Signed-off-by: zjgarvey <[email protected]>
@zjgarvey zjgarvey marked this pull request as ready for review October 21, 2025 02:11
@zjgarvey
Copy link
Collaborator Author

cc: @raayandhar

Copy link
Member

@sahas3 sahas3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some minor nits. Thanks for fixing this issue.

@zjgarvey
Copy link
Collaborator Author

Sorry this is taking a bit to land.

I'm a bit skeptical about this approach, specifically because of the version difference for pybind11 between pytorch and us, so I'm looking a bit deeper into what is going on and trying to make sure this isn't just a lucky hack or something.

@zjgarvey
Copy link
Collaborator Author

zjgarvey commented Oct 21, 2025

Ah, it looks like we include pybind headers from torch instead of our own pybind pip install, which is why this ends up working. The only think I'd want to be sure of is the use of libstdc++ vs. libc++. Torch is compiled with GCC and uses libstdc++. I'd expect we are using libc++, since I'm compiling the jit ir importer with clang, but it appears to be the former from inspecting the pybind11_interals_* symbol in the cpython.so file. So I'm a bit confused there, and I'm not sure if this is a coincidence specific to my system or if this is specified explicitly somewhere.

Edit: I think I'm wrong about the default libraries used by clang on linux. I think it actually defaults to libstdc++, so I think it's fine as is. I'll update the comments and land this PR.

@zjgarvey zjgarvey requested a review from sahas3 October 21, 2025 21:43
This change also more accurately screens for when `torch._C` doesn't have the attribute `_PYBIND11_BUILD_ABI`.

Signed-off-by: zjgarvey <[email protected]>
Signed-off-by: zjgarvey <[email protected]>
@zjgarvey zjgarvey merged commit 7e1cd8b into main Oct 22, 2025
3 checks passed
@zjgarvey zjgarvey deleted the users/zjgarvey/update_pybind11 branch October 22, 2025 00:58
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

Successfully merging this pull request may close these issues.

Build failures for new torch stable 2.9.0

3 participants