search

storpipfugl / pykdtree

Fast kd-tree implementation in Python
GNU Lesser General Public License v3.0
215 stars 47 forks source link

Fix "conflicting declaration" errors #65

Closed pbsds closed 2 years ago

djhoese commented 2 years ago

Could you paste here a copy of the full error message you are getting? Why does the typedef/struct need to be defined this way? I thought both ways were valid?

pbsds commented 2 years ago

I've been playing around with OccNet with various versions of Pytorch. It includes a vendored copy of project, which it compiles with torch.utils.cpp_extension.BuildExtension. Pytorch 1.7.0 for some reason uses g++ instead of gcc, which likely was the issue. I've been unable to reproduce this issue for older and newer versions of pytorch.

Wether you want to merge this or not is up to you, but it's likely not required.

djhoese commented 2 years ago

Were the "conflicting declaration" errors or warnings? I wonder if pytorch's cpp_extension.BuildExtension is only meant to build C++ extensions as the name suggest and it is a little too strict for C extensions. I don't see a reason not to merge this PR as it is mostly a style thing from my perspective, but I'd still like to know all the details for future reference.

Thanks for making the PR. Any other info you can provide is appreciated.

pbsds commented 2 years ago

Error, and it was most likely a bug in cpp_extension, as i'm able to reproduce it like so: image

With this PR, and an additional #define restrict __restrict__, it compiles under g++, although the resulting shared object are missing the correct symbols for python to be able to import it, likely due to a lacking extern "c" or something.

They hide it, but cpp_extension do support C extensions as well:

image source: https://pytorch.org/tutorials/advanced/cpp_extension.html

djhoese commented 2 years ago

I'm not sure I fully understand who is in the right or wrong here and I was fine merging this, but if you don't think it should be then OK.