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

Copy headers to prefix/include #19

Closed
wants to merge 1 commit into from
Closed

Conversation

ELundby45
Copy link

@ELundby45 ELundby45 commented Jan 23, 2025

pybind11 rebuild

Changes

@JeanChristopheMorinPerso

@ELundby45
Copy link
Author

@ELundby45 From what I see in https://conda-metadata-app.streamlit.app/?q=pkgs%2Fmain%2Fwin-64%2Fpybind11-global-2.13.6-py39h214f63a_0.conda, the headers should already be there no?

I don't think so? I see:

Library/include/pybind11/attr.h

and 

include/pybind11_global/pybind11/attr.h

But what I need is:

include/pybind11/attr.h

@JeanChristopheMorinPerso
Copy link

JeanChristopheMorinPerso commented Jan 24, 2025

include/pybind11/attr.h is "broken" though... Files on Windows should go in Library (in the conda ecosystem).

@danpetry
Copy link

a sub-system of pytorch is expecting pip-like include directories. alternative is to patch pytorch to expect more conda-like directory structure. I prefer this solution as it's more broadly applicable, but either will do imho (although patching pytorch will be harder)

@danpetry
Copy link

danpetry commented Jan 24, 2025

it occurs when you define custom pytorch operators, possibly specific to automatic gradient calculation. The error hasn't cropped up in the general case.

@JeanChristopheMorinPerso

The thing is that all packages in uou ecosystem look for stuff in Library. So the problem is not with pybind11. If pytorch looks for stuff in $PREFIX instead of LIBRARY_PREFIX on Windows, then something is wrong and other stuff is probably broken.

@JeanChristopheMorinPerso

Basically, my concern is that merging this change could potentially hide very important problems in our ecosystem.

@danpetry
Copy link

danpetry commented Jan 24, 2025

in the rest of the ecosystem or just pytorch?

Yea, probably best to make sure pytorch is looking in the right place at runtime. Otherwise the jit functions might not work somewhere else either.

@JeanChristopheMorinPerso

in the rest of the ecosystem or just pytorch?

Saying ecosystem was intentional. If something in pytorch is not able to find the right files where they should usually be, it could well be an ecosystem problem or a pytorch problem. By "fixing" the problem in pybind11, you risk making the problem bleed into potentially the rest of the ecosystem.

@ELundby45 ELundby45 closed this Jan 24, 2025
@ELundby45 ELundby45 deleted the rebuild/relocate-headers branch January 24, 2025 16:41
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.

3 participants