scipy / scipy

SciPy library main repository
https://scipy.org
BSD 3-Clause "New" or "Revised" License
12.89k stars 5.13k forks source link

scipy.fftpack.hilbert differs from textbook definition (Trac #1506) #2031

Closed scipy-gitbot closed 11 years ago

scipy-gitbot commented 11 years ago

Original ticket http://projects.scipy.org/scipy/ticket/1506 on 2011-09-01 by trac user xinchuan, assigned to unknown.

from function documentation, scipy implements: y_j = sqrt(-1)sign(j) \ x_j

however, textbook definition: y_j = -1 * sqrt(-1)sign(j) \ x_j

scipy-gitbot commented 11 years ago

@rgommers wrote on 2011-09-04

The difference is only a minus sign, and the Wikipedia article on the Hilbert transform mentions that different books use conflicting definitions (with and without minus sign). So I think the right thing to do is to document this more clearly and point out that the user may need to add a minus sign. Changing the code breaks backwards compatibility for a very minor gain.

Can you confirm that the documentation and implementation actually agree?

scipy-gitbot commented 11 years ago

trac user xinchuan wrote on 2011-09-04

Yes, the documentation and implementation do agree for scipy.fftpack.hilbert. i.e. H(w) = +1j*sign(w)

Note however that there is another function "scipy.signal.hilbert" which computes the analytic signal. Its documentation says "x_a = F^{-1}(F(x) 2U) = x + i y" where "y is the Hilbert transform of x". The "x + i y" follows the H(w) = -1j*sign(w) convention.

So mainly the confusion for the user would be that 2 differing conventions are used within scipy. i.e. signal.hilbert(x) == x - 1j * fftpack.hilbert(x) as currently implemented.

I suspect fftpack.hilbert is not used so much directly. I am more interested in the analytical signal than computing the hilbert transform as an intermediate. FWIW, "Discrete Time Signal Processing by Oppenheim and Schafer" use H(w) = -1j*sign(w)

scipy-gitbot commented 11 years ago

@rgommers wrote on 2011-09-06

Okay, thanks for checking. Probably still best to document (in both functions docstrings) rather than change the behavior.

scipy-gitbot commented 11 years ago

Milestone changed to 0.10.0 by @rgommers on 2011-09-06

scipy-gitbot commented 11 years ago

@rgommers wrote on 2011-09-11

Documented in 9001996 for fftpack.hilbert. Since the usual convention is followed in signal.hilbert I didn't mention it there.