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

require fftw 3.3.9 and use planner_nthreads in finufft.cpp #521

Open
ahbarnett opened this issue Aug 13, 2024 · 3 comments · May be fixed by #526
Open

require fftw 3.3.9 and use planner_nthreads in finufft.cpp #521

ahbarnett opened this issue Aug 13, 2024 · 3 comments · May be fixed by #526

Comments

@ahbarnett
Copy link
Collaborator

ahbarnett commented Aug 13, 2024

p->fftwPlan = FFTW_PLAN_MANY_DFT(dim, ns, p->batchSize, (FFTW_CPX *)p->fwBatch,

rewrite that fftw plan call using fftw_planner_nthreads.
See https://www.fftw.org/release-notes.html
3.3.9 was released Dec 2020.

Julia crash may be related to this:
ludvigak/FINUFFT.jl#61

@jkrimmer jkrimmer linked a pull request Aug 19, 2024 that will close this issue
@jkrimmer
Copy link
Contributor

jkrimmer commented Aug 19, 2024

Hi Alex,

I have implemented a first draft to use fftw_planner_nthreads at #526. Based on this PR, I have built the finufft library for Julia on Windows. Although, the library seems to work fine, the issue described in ludvigak/FINUFFT.jl/issues/61 persists.

Since FFTW.jl and FINUFFT.jl share the same libfftw(f) in julia, I have used the following commands to veriy that this PR works:

With finufft v2.2.0:

julia> @info FFTW.get_num_threads() # calls fftw_planner_nthreads internally
[ Info: 1

julia> nufft1d1(rand(1024), rand(ComplexF64, 1024), 1, 1e-6, 1024; debug=true, nthreads=4);
[finufft_makeplan] new plan: FINUFFT version 2.2.0 .................
[finufft_makeplan] 1d1: (ms,mt,mu)=(1024,1,1) (nf1,nf2,nf3)=(2048,1,1)
               ntrans=1 nthr=4 batchSize=1
[finufft_makeplan] kernel fser (ns=7):          0.000552 s
[finufft_makeplan] fwBatch 0.00GB alloc:        3.91e-06 s
[finufft_makeplan] FFTW plan (mode 64, nthr=4): 0.000163 s
[finufft_setpts] sort (didSort=1):              8.41e-05 s
[finufft_execute] start ntrans=1 (1 batches, bsize=1)...
[finufft_execute] done. tot spread:             0.000381 s
               tot FFT:                         0.00301 s
               tot deconvolve:                  1e-05 s

julia> @info FFTW.get_num_threads()
[ Info: 4

With finufft build from the PR

julia> @info FFTW.get_num_threads() # calls fftw_planner_nthreads internally
[ Info: 1

julia> nufft1d1(rand(1024), rand(ComplexF64, 1024), 1, 1e-6, 1024; debug=true, nthreads=4);
[finufft_makeplan] new plan: FINUFFT version 2.3.0-rc1 .................
[finufft_makeplan] 1d1: (ms,mt,mu)=(1024,1,1) (nf1,nf2,nf3)=(2048,1,1)
               ntrans=1 nthr=4 batchSize=1
[finufft_makeplan] kernel fser (ns=7):          0.000106 s
[finufft_makeplan] fwBatch 0.00GB alloc:        1.11e-06 s
[finufft_makeplan] FFTW plan (mode 64, nthr=4): 5.99e-05 s
[finufft_setpts] sort (didSort=1):              8.9e-05 s
[finufft_execute] start ntrans=1 (1 batches, bsize=1)...
[finufft_execute] done. tot spread:             0.000491 s
               tot FFT:                         7.11e-05 s
               tot deconvolve:                  1.31e-05 s

julia> @info FFTW.get_num_threads()
[ Info: 1

Cheers
Jonas

@DiamonDinoia
Copy link
Collaborator

It seems more of a FFTW.jl issue JuliaMath/FFTW.jl#306. Shall we delay this to 2.4.0 or 2.3.1, in case we do a bugfix release? More investigation is needed in any case.

@ahbarnett
Copy link
Collaborator Author

Robert has created a longer-term solution in the form of a user lock (adding 3 opts fields):
https://github.com/blackwer/finufft/tree/user_fftw_lock

Propose we assess that for 2.4 and extend it to language wrappers.

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 a pull request may close this issue.

3 participants