add FFT=DUCC option to makefile #511

ahbarnett · 2024-08-02T03:22:10Z

Implements switchable FFT in gnu makefile.

Still to do:

docs to match
python test

This also fixes a couple of indep makefile bugs:

mat/oct R2008OO directive always used, not just when multithreaded
examples threadsafe shoudn't build when OMP=OFF (broke).

Reviewers please could you check:

DUCC_CXXFLAGS := -fPIC -std=c++17 -ffast-math I just took from cmake/setupDUCC but I'm not sure if the best default flags, esp, given fastmath bleed-over recent issue.
make objclean Windows I did not attempt
OSX I haven't checked it works yet.

The full set of test cases really is the Cartesian product:

make all
make all FFT=DUCC
make all OMP=OFF
make all FFT=DUCC OMP=OFF

WOuld be nice to have all tested in CI some day.

… to ducc_setup (kill this)

… and crashed at runtime)

makefile

DiamonDinoia · 2024-08-02T15:20:42Z

the following command fails for me:

make clean && make test FFT=DUCC

error:

include/finufft/fft.h:5:10: fatal error: ducc0/fft/fftnd_impl.h: No such file or directory
    5 | #include "ducc0/fft/fftnd_impl.h"

Doing

make clean && make setup FFT=DUCC && make test FFT=DUCC

works fine. Maybe the COOKIE is not picked up properly?

DiamonDinoia · 2024-08-02T15:30:00Z

CI failure seems to a missing openmp flag while compiling.

… into makeducc

ahbarnett · 2024-08-02T20:34:20Z

great catch, and thanks for the fix, Libin.
I forgot about fft.h in defs.h in the spreader... simply needed since defs.h includes the plan object (which may include FFTW plan). Maybe templating whole thing would fix this, so I won't change it.

Meanwhile I've done (and improved) the install docs. Will PR soon.

DiamonDinoia · 2024-08-05T15:05:23Z

I have the following issue, if I do

make clean && make setupclean && make test FFT=DUCC -j

then some compile commands are executed before clone finishes and I get the following errors:

Cloning repository https://github.com/xtensor-stack/xsimd.git at tag 13.0.0 into directory deps/xsimd
Cloning repository https://gitlab.mpcdf.mpg.de/mtr/ducc.git at tag ducc0_0_34_0 into directory deps/ducc
make: *** No rule to make target 'deps/ducc/src/ducc0/infra/string_utils.o', needed by 'lib/libfinufft.so'.  Stop.
make: *** Waiting for unfinished jobs....
Cloning into 'deps/ducc'...
Cloning into 'deps/xsimd'...
In file included from src/fft.cpp:2:
include/finufft/fft.h:5:10: fatal error: ducc0/fft/fftnd_impl.h: No such file or directory
    5 | #include "ducc0/fft/fftnd_impl.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~

I encountered that issue with xsimd in the past and had to change the dependencies structure for all the files requiring xsimd to depend on the .PHONY setup

ahbarnett · 2024-08-05T15:07:37Z

I am aware of this. The ducc sources need to have the ducc setup target as a dependency...

…

On Mon, Aug 5, 2024 at 11:05 AM Marco Barbone ***@***.***> wrote: I have the following issue, if I do make clean && make setupclean && make test FFT=DUCC -j then some compile commands are executed before clone finishes and I get the following errors: Cloning repository https://github.com/xtensor-stack/xsimd.git at tag 13.0.0 into directory deps/xsimd Cloning repository https://gitlab.mpcdf.mpg.de/mtr/ducc.git at tag ducc0_0_34_0 into directory deps/ducc make: *** No rule to make target 'deps/ducc/src/ducc0/infra/string_utils.o', needed by 'lib/libfinufft.so'. Stop. make: *** Waiting for unfinished jobs.... Cloning into 'deps/ducc'... Cloning into 'deps/xsimd'... In file included from src/fft.cpp:2: include/finufft/fft.h:5:10: fatal error: ducc0/fft/fftnd_impl.h: No such file or directory 5 | #include "ducc0/fft/fftnd_impl.h" | ^~~~~~~~~~~~~~~~~~~~~~~~ I encountered that issue with xsimd in the past and had to change the dependencies structure for all the files requiring xsimd to depend on the .PHONY setup — Reply to this email directly, view it on GitHub <#511 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNZRSRVJDSD5FY674EZI3TZP6IEVAVCNFSM6AAAAABL3XOYU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRZGI4TQNJSGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

DiamonDinoia · 2024-08-05T15:40:51Z

I tried:

$(DUCC_SRC)/%.cc: $(DUCC_COOKIE)
$(DUCC_SRC)/%.o: $(DUCC_SRC)/%.cc

or

$(DUCC_SRC)/%.cc: setup
$(DUCC_SRC)/%.o: $(DUCC_SRC)/%.cc

but it does not seem to work

ahbarnett · 2024-08-05T15:42:46Z

Weird. We could just add a note to do make setup without -j, then proceed with -j if they want.

I have other deadlines until about 5pm but can help then. There's probably some remaining doc tweaks, but it should be ready by tomorrow meeting.

lu1and10 · 2024-08-05T22:06:38Z

Maybe let's make it work first, I made an ugly commit to make makefile work with -j first.

split ducc_setup to fft.h dependency.
expand the following wildcard(not working) explicitly for ducc source, not sure why it's not working with the wildcard though, need to clean up the redundancy to use the correct wildcard.

$(DUCC_SRC)/%.cc: $(DUCC_SETUP)
$(DUCC_SRC)/%.o: $(DUCC_SRC)/%.cc
        $(CXX) -c $(DUCC_CXXFLAGS) $(DUCC_INCL) $< -o $@

… into makeducc

ahbarnett · 2024-08-06T17:12:35Z

Thanks, Libin, for the (ugly) fix and the helpful comments in the makefile.
It works my end. I'm just making a bit of doc tidying then the PR is close...

ahbarnett · 2024-08-06T17:21:43Z

I'm hoping Marcos' FrpeoLT was a local editing problem - there is no way the file utils.h could have been changed like that!

ahbarnett · 2024-08-06T18:49:29Z

ready to go. (except I can't get python to build locally, and can't doc it, indep issue.)

DiamonDinoia · 2024-08-06T19:11:37Z

CMakeLists.txt

+  # include(cmake/setupSphinx.cmake)    # to be made default off since only for
+  # devs


This should be not commented out. Please, merge master

DiamonDinoia · 2024-08-06T19:12:56Z

docs/devnotes.rst

@@ -49,7 +49,9 @@ Developer notes

 * The cufinufft Python wheels are generated using Docker based on the manylinux2014 image. For instructions, see ``tools/cufinufft/distribution_helper.sh``. These are binary wheels that are built using CUDA 11 (or optionally CUDA 12, but these are not distributed on PyPI) and bundled with the necessary libraries.

-* Testing cufinufft (for FI, mostly)
+* CMake compiling on linux at Flatiron Institute (Rusty cluster): We have had a report that if you want to use LLVM, you need to ``module load llvm/16.0.3`` otherwise the default ``llvm/14.0.6`` does not find ``OpenMP_CXX``.


maybe this is outdated? @blackwer, could you give a review?

makefile

ahbarnett · 2024-08-06T21:49:06Z

trouble since I deleted cmake/ folder and then did new commits, then pushed. Trying to recover it.

DiamonDinoia · 2024-08-06T21:49:36Z

git checkout master -- cmake should do

ahbarnett · 2024-08-06T21:51:06Z

It sees them as added files but refuses to push them...

ahbarnett · 2024-08-06T21:59:25Z

Ok, pushed. I am getting really messed up by this new clang-format hook. (it means I have to commit twice).
Also it has messed up the cmake/setupXSIMD.cmake and cmake/setupCPM.cmake and won't stop reformatting them.. This clang-format is a huge headache for me...

ahbarnett · 2024-08-06T22:00:23Z

Marco, could you kindly fix those cmake files- it is impossible for me to do it thanks to your clang-format...

DiamonDinoia · 2024-08-06T22:01:41Z

I will have a look.

ahbarnett · 2024-08-06T22:06:04Z

doesn't seem to affect the CI. I know I have to edit .pre-commit-config.yaml
If there's an edit which stops it formatting cmake/* or *.cmake would be great. THanks!

ahbarnett · 2024-08-06T22:09:02Z

I'm bringing in and fixing post-merge.

lu1and10 · 2024-08-07T01:37:39Z

@ahbarnett wildcard for ducc sources seems to work in the commit 57cd182 let me know if it breaks on your side

ahbarnett · 2024-08-07T01:50:07Z

Seems to work! Thanks! I don't understand the repeated colons though (two colons on same line? I didn't see that in the make manual...) Cheers, Alex

…

On Tue, Aug 6, 2024 at 9:39 PM Libin Lu ***@***.***> wrote: @ahbarnett <https://github.com/ahbarnett> wildcard for ducc sources seems to work in the commit 57cd182 <57cd182> let me know if it breaks on your side — Reply to this email directly, view it on GitHub <#511 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNZRSTPXC4I4AWMPQDZU5DZQF27TAVCNFSM6AAAAABL3XOYU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZSGQ2TKMRYGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

lu1and10 · 2024-08-07T01:51:35Z

Seems to work! Thanks! I don't understand the repeated colons though (two colons on same line? I didn't see that in the make manual...) Cheers, Alex

I learn from this http://makefiletutorial.com/#static-pattern-rules
Seems a popular tutorial, having 4.7k git stars.

ahbarnett · 2024-08-07T01:55:30Z

Cute!

…

On Tue, Aug 6, 2024 at 9:52 PM Libin Lu ***@***.***> wrote: Seems to work! Thanks! I don't understand the repeated colons though (two colons on same line? I didn't see that in the make manual...) Cheers, Alex … <#m_-3368767352935253467_> On Tue, Aug 6, 2024 at 9:39 PM Libin Lu *@*.*> wrote: @ahbarnett <https://github.com/ahbarnett> https://github.com/ahbarnett <https://github.com/ahbarnett> wildcard for ducc sources seems to work in the commit 57cd182 <57cd182> <57cd182 <57cd182>> let me know if it breaks on your side — Reply to this email directly, view it on GitHub <#511 (comment) <#511 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNZRSTPXC4I4AWMPQDZU5DZQF27TAVCNFSM6AAAAABL3XOYU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZSGQ2TKMRYGY <https://github.com/notifications/unsubscribe-auth/ACNZRSTPXC4I4AWMPQDZU5DZQF27TAVCNFSM6AAAAABL3XOYU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZSGQ2TKMRYGY> . You are receiving this because you were mentioned.Message ID: @.*> -- *-------------------------------------------------------------------^`^._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942 I learn from this http://makefiletutorial.com/#static-pattern-rules Seems a popular, having 4.7k git stars. — Reply to this email directly, view it on GitHub <#511 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNZRSS4VPSAFGXXW5QOZB3ZQF4T5AVCNFSM6AAAAABL3XOYU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZSGQ3DMOJYGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- *-------------------------------------------------------------------~^`^~._.~' |\ Alex Barnett Center for Computational Mathematics, Flatiron Institute | \ http://users.flatironinstitute.org/~ahb 646-876-5942

ahbarnett added 5 commits August 1, 2024 18:08

ducc makefile kinda working, still rebuilds lib for every example due…

486ae6a

… to ducc_setup (kill this)

make FFT=DUCC working

69fd51b

matlab lgomp, needed for DUCC FFT

155499e

moved mat/oct flags R2008OO to always apply (was not used for OMP=OFF…

1cfeeb2

… and crashed at runtime)

make examples stop threadsafe tests when OMP=OFF

f0aaf71

ahbarnett mentioned this pull request Aug 2, 2024

add makefile (and document) DUCC FFT option #479

Closed

9 tasks

lu1and10 reviewed Aug 2, 2024

View reviewed changes

makefile Outdated Show resolved Hide resolved

ahbarnett and others added 6 commits August 2, 2024 12:52

Merge branch 'master' into makeducc

f8b356a

try to make ci for mac clang work and setup ducc

7ecc0c1

tidy changelog

5ac746a

cleaned up docs/install re CMake DUCC

890ff44

better gnu make docs, include all tasks/options, including DUCC

8988f1d

Merge branch 'makeducc' of https://github.com/flatironinstitute/finufft…

1865324

… into makeducc

let's make -j work with makefile first, need to clean up

f1875fa

Merge branch 'makeducc' of https://github.com/flatironinstitute/finufft…

cbed1a4

… into makeducc

edit/rationalize docs/install.rst

11d8889

ahbarnett mentioned this pull request Aug 6, 2024

Update building documentation #506

Open

6 tasks

attempt to doc Joakim's pyproject.toml system

f497e22

DiamonDinoia reviewed Aug 6, 2024

View reviewed changes

makefile Show resolved Hide resolved

DiamonDinoia reviewed Aug 6, 2024

View reviewed changes

makefile Show resolved Hide resolved

ahbarnett added 3 commits August 6, 2024 17:19

remove troublesome -e from make python; merge CMakeLists.txt

32138cd

update python install docs to remove -e

5441880

merge CHANGELOG and tidy its GPU list

a8c3a8b

cmake/ restore

f41ae29

ahbarnett merged commit 353c971 into master Aug 6, 2024
332 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add FFT=DUCC option to makefile #511

add FFT=DUCC option to makefile #511

ahbarnett commented Aug 2, 2024 •

edited

Loading

DiamonDinoia commented Aug 2, 2024 •

edited

Loading

DiamonDinoia commented Aug 2, 2024

ahbarnett commented Aug 2, 2024

DiamonDinoia commented Aug 5, 2024

ahbarnett commented Aug 5, 2024 via email

DiamonDinoia commented Aug 5, 2024 •

edited

Loading

ahbarnett commented Aug 5, 2024 •

edited

Loading

lu1and10 commented Aug 5, 2024 •

edited

Loading

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

DiamonDinoia Aug 6, 2024

DiamonDinoia Aug 6, 2024 •

edited

Loading

ahbarnett commented Aug 6, 2024

DiamonDinoia commented Aug 6, 2024 •

edited

Loading

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

DiamonDinoia commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

lu1and10 commented Aug 7, 2024

ahbarnett commented Aug 7, 2024 via email

lu1and10 commented Aug 7, 2024 •

edited

Loading

ahbarnett commented Aug 7, 2024 via email

		# include(cmake/setupSphinx.cmake) # to be made default off since only for
		# devs

add FFT=DUCC option to makefile #511

add FFT=DUCC option to makefile #511

Conversation

ahbarnett commented Aug 2, 2024 • edited Loading

DiamonDinoia commented Aug 2, 2024 • edited Loading

DiamonDinoia commented Aug 2, 2024

ahbarnett commented Aug 2, 2024

DiamonDinoia commented Aug 5, 2024

ahbarnett commented Aug 5, 2024 via email

DiamonDinoia commented Aug 5, 2024 • edited Loading

ahbarnett commented Aug 5, 2024 • edited Loading

lu1and10 commented Aug 5, 2024 • edited Loading

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

DiamonDinoia Aug 6, 2024

Choose a reason for hiding this comment

DiamonDinoia Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

ahbarnett commented Aug 6, 2024

DiamonDinoia commented Aug 6, 2024 • edited Loading

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

DiamonDinoia commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

ahbarnett commented Aug 6, 2024

lu1and10 commented Aug 7, 2024

ahbarnett commented Aug 7, 2024 via email

lu1and10 commented Aug 7, 2024 • edited Loading

ahbarnett commented Aug 7, 2024 via email

ahbarnett commented Aug 2, 2024 •

edited

Loading

DiamonDinoia commented Aug 2, 2024 •

edited

Loading

DiamonDinoia commented Aug 5, 2024 •

edited

Loading

ahbarnett commented Aug 5, 2024 •

edited

Loading

lu1and10 commented Aug 5, 2024 •

edited

Loading

DiamonDinoia Aug 6, 2024 •

edited

Loading

DiamonDinoia commented Aug 6, 2024 •

edited

Loading

lu1and10 commented Aug 7, 2024 •

edited

Loading