flatironinstitute / finufft

Non-uniform fast Fourier transform library of types 1,2,3 in dimensions 1,2,3
Other
303 stars 79 forks source link

makefile: pass DUCC option to Python sk-build via CMAKE_ARGS in pip #586

Closed ahbarnett closed 3 weeks ago

ahbarnett commented 1 month ago

on my linux system, this fixes #585

Should be platform-indep - could someone else check it works?

lu1and10 commented 1 month ago

I tested on my local Mac, I need to do the following tweak to make make FFT=DUCC python work.

diff --git a/makefile b/makefile
index 7ba042bc..822f7006 100644
--- a/makefile
+++ b/makefile
@@ -117,9 +117,10 @@ ifneq ($(OMP),OFF)
   MFLAGS += $(MOMPFLAGS)
   OFLAGS += $(OOMPFLAGS)
   LIBS += $(OMPLIBS)
+  LIBSFFT += $(OMPLIBS)
 # fftw3 multithreaded libs...
   ifneq ($(FFT),DUCC)
-    LIBSFFT += -l$(FFTWNAME)_$(FFTWOMPSUFFIX) -l$(FFTWNAME)f_$(FFTWOMPSUFFIX) $(OMPLIBS)
+    LIBSFFT += -l$(FFTWNAME)_$(FFTWOMPSUFFIX) -l$(FFTWNAME)f_$(FFTWOMPSUFFIX)
   endif
 endif

diff --git a/python/finufft/pyproject.toml b/python/finufft/pyproject.toml
index 68f08b8f..3ce0d2e2 100644
--- a/python/finufft/pyproject.toml
+++ b/python/finufft/pyproject.toml
@@ -11,7 +11,7 @@ build-backend = "scikit_build_core.build"
 name = "finufft"
 readme = "README.md"
 requires-python = ">=3.8"
-dependencies = ["numpy >= 1.12.0"]
+dependencies = ["numpy >= 1.12.0", "packaging"]
 authors = [
     {name = "Jeremy Magland"},
     {name = "Daniel Foreman-Mackey"},

The makefile diff is independent of python, in order to use Apple Clang + libomp, I need to explicit link libomp(with gcc it's fine, gcc seems to link libgomp even no -lgomp is specified). Current makefile logic for DYNLIB uses OMPFLAGS and LIBSFFT, while if DUCC is on and OMP is ON, the omp lib is not appended to LIBSFFT

For the pyproject.toml diff. Despite of the commit https://github.com/flatironinstitute/finufft/commit/9e88e801e995953a20fa56c02a7212dbdf8f7b62, on my local Mac with a brand new empty python env, I need to add the packaging requirement for python in pyproject.toml too in order to make make FFT=DUCC python work. The command installs finufft successfully, just when importing finufft, the packaging needs to be installed too. Adding packaging to pyproject.toml makes packaging installed automatically when installing finufft.

mreineck commented 1 month ago

For me the patch works fine (Debian unstable), thanks!