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

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

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

ahbarnett
Copy link
Collaborator

on my linux system, this fixes #585

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

@lu1and10
Copy link
Member

lu1and10 commented Oct 25, 2024

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 9e88e80, 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
Copy link
Collaborator

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

@ahbarnett ahbarnett merged commit f627567 into master Nov 1, 2024
331 of 332 checks passed
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.

"make python FFT=DUCC" uses FFTW
4 participants