search

matteo-ronchetti / torch-radon

Computational Tomography in PyTorch
https://torch-radon.readthedocs.io
GNU General Public License v3.0
219 stars 45 forks source link

Use buildsystem provided by pytorch #37

Closed carterbox closed 9 months ago

carterbox commented 2 years ago

I don't understand the reason for the custom python/make build system in ptx_annotation.py and make.py, but it was causing some missing symbol errors for me when I tried to load the compiled python module.

In this PR, I replaced those scripts with the CUDAExtension builder provided by pytorch. I found that using this build system is probably the best option if the goal is to produce a python pytorch extension module because all the linking, headers, cuda capability selection, compiler args, and linking args are set automatically to be compatible with pytorch. It also has some useful error messages about compiler compatibility.

As noted, in setup.py I did need to change some arguments from the pytorch defaults in order to support the hexfloats used in rmath.h and __half conversions which pytorch prohibits (for reasons unknown to me).

I did read implementation.md which states that the goal is to have not only a python interface, but also a c/c++ interface for the lower level functions of this library. Maybe that is one of the reasons for the custom build script? Is anyone actually using that low level API? If so, I would be happy to help implement a CMake build system for a cxx library that is compiled separately from (and then linked to?) the python module. However, the C/CXX interface needs clear separation from the pytorch interface (mostly just by separating the source files clearly).

This branch also contains some sanity checks that I added for the ParallelRadon module, but those could be removed from this PR.

carterbox commented 2 years ago

Marked this PR as ready for review because I've been using it for a few weeks now and haven't wanted to change anything. It needs review feedback scope.