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

DOC: Tweaks to fft documentation #10371

Merged
merged 4 commits into from Jul 2, 2019
Merged

DOC: Tweaks to fft documentation #10371

merged 4 commits into from Jul 2, 2019

Conversation

peterbell10
Copy link
Member

This fixes the typos pointed out by @endolith in #10238 and updates the scipy.fftpack documentation to link to scipy.fft.rfft instead of the NumPy version.

@endolith
Copy link
Member

Ok, https://github.com/scipy/scipy/blob/master/scipy/fft/_basic.py should be updated to mention that it uses Bluesteins's algorithm now?

Are the speed benefits of fft < rfft < dct still true?

@peterbell10
Copy link
Member Author

Are the speed benefits of fft < rfft < dct still true?

  • fft <= rfft is still true since rfft falls back to a complex bluestein transform for prime sizes.
  • dct is still using fftpack so will be slower for prime sizes.

@endolith
Copy link
Member

The 13× speedup example on https://13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fft.next_fast_len.html is no longer correct, right?

It also says

(These are also known as n-smooth numbers, regular numbers, or Hamming numbers.)

"n-smooth" applies to any n, of course, but "regular numbers" and "Hamming numbers" only applies to 5-smooth. https://en.wikipedia.org/wiki/Regular_number

@peterbell10
Copy link
Member Author

I redid the timings with pypocketfft. Fftpack was something like 600x slower and that was for a much smaller transform.

@endolith
Copy link
Member

Also the references are formatted strangely on https://13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fft.fft.html#id4 but I think the markup isn't to blame?

On https://13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fft.rfftfreq.html
the "(but like scipy.fftpack.rfftfreq)" remark could probably be removed

@endolith
Copy link
Member

Fftpack was something like 600x slower

I don't mean pocketfft vs fftpack, I mean pocketfft with a prime size vs a power of 2 vs a n-smooth size, as in the example text

@peterbell10
Copy link
Member Author

On 13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fft.rfftfreq.html
the "(but like scipy.fftpack.rfftfreq)" remark could probably be removed

The fftfreq functions are all imported directly from numpy.

I don't mean pocketfft vs fftpack, I mean pocketfft with a prime size vs a power of 2 vs a n-smooth size, as in the example text

Yes, that's what I meant as well. fftpack's next_fast_len docs show the prime size as 630x slower than the next_fast_len size.

I retimed everything in the example using scipy.fft, making the fft size much larger so that the difference was actually significant. Compare it with the fftpack version and the numbers are completely different.

@larsoner
Copy link
Member

@endolith can you see if this addresses your concerns?

@pvanmulbregt pvanmulbregt added the Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org label Jul 1, 2019
@endolith
Copy link
Member

endolith commented Jul 1, 2019

Well the description of speeds is still incorrect, right? Can that be updated? The Examples section in https://13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fft.next_fast_len.html ?

@peterbell10
Copy link
Member Author

Well the description of speeds is still incorrect, right? Can that be updated?

It's already up to date. Compare it to the fftpack docs:
https://13717-1460385-gh.circle-artifacts.com/0/html-scipyorg/generated/scipy.fftpack.next_fast_len.html#scipy.fftpack.next_fast_len

@endolith
Copy link
Member

endolith commented Jul 1, 2019

Oh, ok.

@pv pv merged commit d053b57 into scipy:master Jul 2, 2019
@pv pv added this to the 1.4.0 milestone Jul 2, 2019
@peterbell10 peterbell10 deleted the fft-doc branch July 2, 2019 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants