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

RFC: rewritten FFTW interface #1856

Merged
merged 9 commits into from
Jan 2, 2013
Merged

Conversation

stevengj
Copy link
Member

This patch rewrites the FFTW interface as discussed in issues #1617, #1806, and #1805.

The fft(x) function now performs a multidimensional FFT by default (what used to be called fftn). To transform along 0 or more dimensions, do fft(x, region) where region is an iterable set of integers. (Similarly for bfft, ifft, rfft, etcetera.) The functions fftn, fft2, and fft3 are removed.

It also adds a new function p = plan_fft(x) which returns a function p(x) that computes optimized FFTs of a fixed size. plan_fft has optional arguments to specify the region and FFTW planner flags.

It also adds in-place functions fft! etcetera. Subarrays are now supported too.

There are a few other odds and ends that I changed. e.g. FFTW.import_wisdom and FFTW.export_wisdom now import/export both single and double precision wisdom from/to a single file. The FFTW.num_threads function should now work properly even if fft has been previously called. brfft no longer overwrites its input. The 64-bit API is used everywhere. The fft.jl tests are a bit more extensive.

The documentation has not been updated yet; I wanted to get some feedback first. UPDATE: Documentation added.

@JeffBezanson
Copy link
Member

This looks awesome! Very exciting to have this. Thanks.

@ViralBShah
Copy link
Member

This is awesome.

Couple of comments. The tests can start passing, once you replace the use of ifft2 and ifft2 in extras/image.jl.

@ViralBShah
Copy link
Member

Oops - I see you have already fixed the image.jl - I was looking at the commits in wrong order.

@ViralBShah
Copy link
Member

The travis build has passed for both gcc and clang after the last commit, but it is showing yellow on github. Perhaps a travis bug? @staticfloat any ideas?

Looks good to merge otherwise.

@Keno
Copy link
Member

Keno commented Dec 31, 2012

Showing green for me!

@stevengj
Copy link
Member Author

Thanks! Probably should wait to merge until I update the documentation. I also noticed a bug in the treatment of subarrays that I should fix, and add a test case for. I should have an updated patch in a couple of days.

@ViralBShah
Copy link
Member

@loladiro It is not showing green here - https://github.com/JuliaLang/julia/pull/1856/commits

@Keno
Copy link
Member

Keno commented Dec 31, 2012

Hmm, interesting seems like a github bug though, since it shows up correctly in the "Discussion" section for me!

for i in 1:numel(X)
x[i] *= s;
end
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In scale! there is a typo on line 407. It should be X instead of x.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am copying these scale! functions into linalg.jl for use in norm, so may be best to remove them from here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I noticed that. Also, it should return X.

@stevengj
Copy link
Member Author

stevengj commented Jan 2, 2013

Added documentation, bugfixes, etcetera. Should hopefully be good to merge.

One thing I observed: plan execution seems to have an overhead of 30-50us compared to C, which is very noticeable for transforms of small sizes (especially <= 1000 or so). Deleting the assert_applicable argument check does not decrease this significantly. Is there some reason why the ccall overhead would be so high? Or is there some large overhead to calling anonymous functions? @JeffBezanson?

ViralBShah added a commit that referenced this pull request Jan 2, 2013
RFC: rewritten FFTW interface
@ViralBShah ViralBShah merged commit 4bfe51d into JuliaLang:master Jan 2, 2013
@ViralBShah ViralBShah mentioned this pull request Jan 2, 2013
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.

4 participants