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

Window Welch periodogram with Hann window by default. #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

c42f
Copy link

@c42f c42f commented Feb 17, 2017

This choice is used by scipy, and seems less likely than the square
window to produce completely wrong results for novice users. matlab and
octave use the Hamming window, but this works very poorly for signals
with a nontrivial DC component when n != nfft.

Fixes #152, at least for me, but review by someone with a lot more signal processing experience would be most useful.

The tests against reference data will fail here, but can be fixed up if this change is desired.

This choice is used by scipy, and seems less likely than the square
window to produce completely wrong results for novice users.  matlab and
octave use the Hamming window, but this works very poorly for signals
with a nontrivial DC component when `n != nfft`.
tradeoff between high spectral resolution (the square window) and
sensitivity to signals near the noise floor. A window which falls to zero
also reduces the spurious ringing which occurs when ``n!=nfft`` for signals
with large mean.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems misleading - unless I'm misunderstanding something, using a rectangular window should give poor spectral resolution (lots of spectral leakage between bins) vs. the hanning, which should have less leakage. I suppose it depends on whether you mean main lobe width or energy in the sidelobes, but the sidelobes of the rectangular window fall off slowly enough that I definitely wouldn't consider it to have high spectral resolution.

a rectangular window will give you the same sort of ringing if the beginning of the windowed signal differs from the end

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough, I honestly can't remember quite what I was thinking at the time I wrote this, or even the precise detail of what I was doing with this code.

I do know that the default caused me some confusion, and that some discussion of the inherent tradeoffs between different windows would be useful.

Feel free to reword it as you please, or close the PR.

@galenlynch
Copy link
Member

galenlynch commented Oct 27, 2018

I like the idea of using beginner friendly default parameters, and as you mention in #152 it seems equivalent functions in scipy/octave use friendlier defaults than DSP.jl.

The issues your described in #152 may also stem from this function not detrending each data segment, which will cause artifacts when n != nfft and the window is not chosen carefully. As no particular care is taken to make sure that n = nfft by default, and the default window is a boxcar, it seems like the current defaults may not be the best.

In order to make the Welch function better behaved over a range of parameters, it may help to make sure n = nfft by default. Additionally, it may help to add an option to detrend each data segment, which would in turn require changes to ArraySplit which is used to split the input data into segments. Scipy seems to detrend each data segment as well.

@c42f
Copy link
Author

c42f commented Oct 28, 2018

Agreed, all good suggestions.

@JakeZw
Copy link

JakeZw commented Mar 10, 2021

Regarding the comment on high spectral resolution quoted above. If high spectral resolution means high frequency resolution, as seems reasonable, then this statement is accurate. As you can see in http://papers.vibetech.com/Paper64-CompilationofTimeWindowsandLineShapesRPotter.pdf, the main lobe of this window has a width of 1. However its amplitude accuracy in general is very poor. When used for a completely contained transient signal or a signal periodic within the window it is the best window to use. https://d3i5bpxkxvwmz.cloudfront.net/articles/2011/02/27/fundamentals-of-signal-analysis-1298867011.pdf gives a reasonable intuitive understanding. This is the window of choice for burst random modal analysis because the signal is (should be) zero outside the window so is a completely contained transient.

Traditional dynamic signal analysers, DSA's, typically have the rectangular, hanning, and Flattop window. Rectangular gives the best frequency resolution and very poor amplitude resolution, Flattop gives excellent amplitude resolution but poor frequency resolution, and hanning is a computationally efficient compromise. The paper by Potter, referred to above gives a number of other windows that are interesting as does the paper by Nutall, https://pdfs.semanticscholar.org/a1fa/3f3bdcb92e17b01d982e660841630991e868.pdf.

@martinholters
Copy link
Member

Do we want this? If we do, should we just switch the default now (for v0.8) or should we deprecate not specifying a window now and then set the new default v0.9?

(BTW, I've rebased this branch locally but cannot push here. If desired, I can open a new PR with it.)

@JakeZw
Copy link

JakeZw commented Nov 5, 2024

I think the hanning window is the best default. By deprecate, do you mean that the user is forced to specify a window and then in version 0.9 there is a new default? That might be the best to prevent surprises in the results a user gets.

@martinholters
Copy link
Member

By deprecate, do you mean that the user is forced to specify a window and then in version 0.9 there is a new default?

Yes, for a mild form of forcing where a deprecation warning is emitted when no window is specified explicitly, but the current default will still be used. I.e. code will continue to run and give the same results, but emit a warning so that users can switch to explicitly specifying window=nothing (or better, reconsider that choice and explicitly request a more appropriate window). I'll prepare a PR to that effect for further discussion of this route.

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.

welch_pgram is easily misused without a default window
5 participants