flatironinstitute / jax-finufft

JAX bindings to the Flatiron Institute Non-uniform Fast Fourier Transform (FINUFFT) library
Apache License 2.0
80 stars 3 forks source link

Update build-backend to scikit-build-core #19

Closed lgarrison closed 1 year ago

lgarrison commented 1 year ago

This PR updates the PEP 517 build backend from classic scikit-build to modern scikit-build-core. Editable installs weren't working anymore in classic skbuild with modern setuptools, so it seemed best to just port to the modern backend.

I followed this migration guide, and it was all pretty straightforward. I had to bump some version requirements and tweak a few things, but for the most part it was a one-to-one transformation. Both regular and editable installs pass the tests now. Also, both types of installs now put the compiled CPython extension in the package directory in site-packages, rather than the source tree, which seems desirable.

I did notice that OpenMP isn't being used, though; is that intended? Is Jax going to do a higher-level parallelization?

Next steps (for upcoming PRs) are:

dfm commented 1 year ago

I did notice that OpenMP isn't being used, though; is that intended? Is Jax going to do a higher-level parallelization?

You mean we're not currently compiling with OpenMP support? I don't really remember looking into that. Will the current version of finufft with CMake support find it automatically?

lgarrison commented 1 year ago

You mean we're not currently compiling with OpenMP support? I don't really remember looking into that. Will the current version of finufft with CMake support find it automatically?

That's right; at least as I'm compiling it, I get lots of warnings about unknown OpenMP pragmas. We'll see if the new finufft turns it on by default; we obviously want it if Jax isn't doing its own parallelization!