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.

Sign up for GitHub

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

Jump to bottom

Use fftw_planner_nthreads #526

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Use fftw_planner_nthreads #526

wants to merge 4 commits into from

Conversation

jkrimmer
Copy link
Contributor

This pull should close #521.

@jkrimmer jkrimmer mentioned this pull request Aug 19, 2024
Closed
@lu1and10
Copy link
Member

lu1and10 commented Aug 19, 2024 •
edited
Loading

As @DiamonDinoia mentioned in the comment

finufft/src/finufft.cpp

Line 763 in 9f517e3

// there is fftw_version which returns a string, but that's not compile time
of PR #517, we may need to use fftw[f]_version to determine the version number is greater or equal than 3.3.9 to use the planner_nthreads function.
The ci is breaking because some of the ubuntu apt still installs fftw 3.3.8, do we still want to support fftw 3.3.8? If so, fftw[f]_version needs to be called to check fftw version.

@lu1and10 lu1and10 mentioned this pull request Aug 19, 2024
Merged
@DiamonDinoia
Copy link
Collaborator

Can that be checked in cmake? If the correct version is not found in the system we can build it from source. This is not supported in makefile but will allow to fix the issue for everyone.

one way to check in cmake is to write a program that links against fftw and prints the fftw_version to stdout, cmake can read the output an compare it with the right version.

@ahbarnett
Copy link
Collaborator

ahbarnett commented Aug 19, 2024 via email

@DiamonDinoia
Copy link
Collaborator

Yes, we could enforce Ubuntu 24.04. But how about the user-base? Ubuntu 22.04 is still quite popular. Even though fftw 3.3.9 has been out I would expect many to use fftw from the package manager.

@DiamonDinoia
Copy link
Collaborator

Hi Jonas,

Maybe restoring the threads does not restore fftw state. Could you replace FFTW_PLAN_TH with a no-op and see if that fixes it?

I like your idea of trying to restore the previous number of threads so to still support 3.3.6.

Thanks,
Marco

@jkrimmer
Copy link
Contributor Author

Dear all,

Thank you very much for looking into this PR!

@DiamonDinoia I assume you are wondering if FFTW_PLAN_TH is needed at all? As far as I have understood, calling fftw_planner_nthreads only returns the number of currently set threads and does not replace the call of fftw_plan_with_nthreads. Still, I have already build finufft after replacing fftw_plan_with_nthreads by a no-op: At least in julia this change does not have any effect. Maybe this behavior is related to the use of fftw_threads_set_callback in julia? Still, I will conduct further tests tomorrow!

Thanks
Jonas

@jkrimmer
Copy link
Contributor Author

jkrimmer commented Aug 21, 2024 •
edited
Loading

Update: I have set up another branch making use of fftw_planner_nthreads to test if we could get rid of FFTW_PLAN_TH over at rm_plan_with_threads.
Building with

mkdir build && cd build; cmake -DFINUFFT_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release -DFINUFFT_STATIC_LINKING=OFF -DFINUFFT_FFTW_SUFFIX=OpenMP ..; cmake --build . -j

and running OMP_NUM_THREADS=4 test/finufft1d_test 1e6 1e6 1e-6 yields the expected result, i.e., the FFTW plan is single-threaded:

test 1d type 1:                                                                                          
[finufft_makeplan] new plan: FINUFFT version 2.3.0-rc1 .................
[finufft_makeplan] 1d1: (ms,mt,mu)=(1000000,1,1) (nf1,nf2,nf3)=(2000000,1,1)
               ntrans=1 nthr=4 batchSize=1
[finufft_makeplan] kernel fser (ns=7):          0.00252 s
[finufft_makeplan] fwBatch 0.03GB alloc:        9e-06 s
[finufft_makeplan] FFTW plan (mode 64, nthr=1): 0.00275 s
[finufft_setpts] sort (didSort=1):              0.00451 s
[finufft_execute] start ntrans=1 (1 batches, bsize=1)...
[finufft_execute] done. tot spread:             0.0203 s
               tot FFT:                         0.0219 s
               tot deconvolve:                  0.00718 s
        1000000 NU pts to 1000000 modes in 0.0634 s     1.58e+07 NU pts/s
        one mode: rel err in F[370000] is 6.59e-08
test 1d type 2:
[finufft_makeplan] new plan: FINUFFT version 2.3.0-rc1 .................
[finufft_makeplan] 1d2: (ms,mt,mu)=(1000000,1,1) (nf1,nf2,nf3)=(2000000,1,1)
               ntrans=1 nthr=4 batchSize=1
[finufft_makeplan] kernel fser (ns=7):          0.00365 s
[finufft_makeplan] fwBatch 0.03GB alloc:        7e-06 s
[finufft_makeplan] FFTW plan (mode 64, nthr=1): 0.000163 s
[finufft_setpts] sort (didSort=0):              0.00282 s
[finufft_execute] start ntrans=1 (1 batches, bsize=1)...
[finufft_execute] done. tot deconvolve:         0.0123 s
               tot FFT:                         0.0228 s
               tot interp:                      0.00891 s
        1000000 modes to 1000000 NU pts in 0.0507 s     1.97e+07 NU pts/s
        one targ: rel err in c[500000] is 1.87e-07
test 1d type 3:
[finufft_makeplan] new plan: FINUFFT version 2.3.0-rc1 .................
[finufft_makeplan] 1d3: ntrans=1
        M=1000000 N=1000000
        X1=3.14 C1=2 S1=5e+05 D1=8.5e+05 gam1=1.00777 nf1=1259712
[finufft_setpts t3] widcen, batch 0.04GB alloc: 0.00122 s
[finufft_setpts t3] phase & deconv factors:     0.0554 s
[finufft_setpts t3] sort (didSort=1):           0.00307 s
[finufft_setpts t3] inner t2 plan & setpts:     0.0325 s
[finufft_execute t3] start ntrans=1 (1 batches, bsize=1)...
[finufft_execute t3] done. tot prephase:                0.00129 s
                  tot spread:                   0.00742 s
                  tot type 2:                   0.0328 s
                  tot deconvolve:               0.00209 s
        1000000 NU to 1000000 NU in 0.145 s             1.38e+07 tot NU pts/s
        one targ: rel err in F[500000] is 5.93e-07

@ahbarnett
Copy link
Collaborator

ahbarnett commented Aug 21, 2024 via email

@jkrimmer
Copy link
Contributor Author

Dear Alex,

I am sorry for the confusion: My new branch rm_plan_with_threads is indeed only a refined test of the FFTW behavior as I am not interested in having single-threaded FFTs only. Instead, my conclusion is that we cannot avoid calling FFTW_PLAN_TH as in

finufft/src/finufft.cpp

Line 763 in 9f517e3

// there is fftw_version which returns a string, but that's not compile time
if we want FFTW to run with multiple threads. Accordingly, I currently have no idea how we could avoid calling fftw_planner_nthreads if we want to determine the current number of threads used by FFTW.

Thanks a lot
Jonas

@ahbarnett
Copy link
Collaborator

ahbarnett commented Aug 21, 2024 via email

@DiamonDinoia DiamonDinoia added this to the 2.4 milestone Aug 27, 2024
@DiamonDinoia
Copy link
Collaborator

DiamonDinoia commented Sep 11, 2024 •
edited
Loading

Not sure we want to do this as it enforces a recent version of fftw that ubuntu 22.04 does not ship with.

We could potentially check if the function fftw_planner_ntreads exists?

#include <iostream>

#ifdef _WIN32
#include <windows.h>
#else
#include <dlfcn.h>
#endif

bool functionExists(const char* functionName) {
#ifdef _WIN32
    HMODULE handle = GetModuleHandle(NULL); // Get handle to the current executable
    if (!handle) {
        std::cerr << "Error getting module handle: " << GetLastError() << std::endl;
        return false;
    }

    FARPROC func = GetProcAddress(handle, functionName);
    return func != nullptr;
#else
    void* handle = dlopen(nullptr, RTLD_LAZY); // Open the current executable
    if (!handle) {
        std::cerr << "Error opening handle: " << dlerror() << std::endl;
        return false;
    }

    dlerror(); // Clear any existing error

    void* func = dlsym(handle, functionName);
    const char* dlsym_error = dlerror();
    dlclose(handle);

    if (dlsym_error) {
        return false;
    }
    return func != nullptr;
#endif
}

int main() {
    const char* functionName = "myFunction";
    if (functionExists(functionName)) {
        std::cout << "Function " << functionName << " exists." << std::endl;
    } else {
        std::cout << "Function " << functionName << " does not exist." << std::endl;
    }
    return 0;
}

@ahbarnett
Copy link
Collaborator

I agree Ubuntu 22.04 is still very much in use (eg by me) so I think we have to support FFTW 3.3.8 for a while. Thanks Marco for the detect-function code. Things start to get more complicated if we go there, so I think we're stuck for now. If there is user pressure we can revisit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.4
Development

Successfully merging this pull request may close these issues.

require fftw 3.3.9 and use planner_nthreads in finufft.cpp
4 participants
@jkrimmer @lu1and10 @DiamonDinoia @ahbarnett