search

raphaelvallat / entropy

EntroPy: complexity of time-series in Python (DEPRECATED)
https://raphaelvallat.com/entropy/
BSD 3-Clause "New" or "Revised" License
155 stars 47 forks source link

Spectral entropy not stable #10

Open MaxHeuillet opened 3 years ago

MaxHeuillet commented 3 years ago

Hello Raphael,

I analyzed the formula for the spectral entropy.

You provide 2 methods: periodogram and welch

However, periodogram is unstable and leads sometimes to Nan/Infinity due to the log2 computation. In order to improve this formula, I would advice you to:

1) add a note for the use of periodogram 2) add a default value to 1 for the sampling frequency

Best, -MH

raphaelvallat commented 3 years ago

Hi @MaxHeuillet!

Ok for the periodogram, could you say a bit more why you think it's unstable and what exactly you would put in the documentation (or please feel free to submit a PR). As for the sampling frequency, why would you put it to 1 Hz? I think it's much better to not use a default value, instead, users should know the sampling frequency of their data.

Thanks, Raphael

MaxHeuillet commented 3 years ago

Hello,

In my case, I manipulate many sequential databases and as far as I know, None of them was provided a sampling frequency...

Do you know what is the implication of using the default value = 1 Hz when one doesn't know the sampling frequency?

As for the periodogram method, I just used it and it lead sometimes to Zero Division errors in the np.lg2 implying generation of Nan values, whereas the Welth worked perfectly on all my databases. I read the following:

Welch's method is an improvement on the standard periodogram spectrum estimating method and on Bartlett's method, in that it reduces noise in the estimated power spectra in exchange for reducing the frequency resolution.

I spent time investigating and checking for your function, I felt it would be nice to give a feedback about my experience! :)

Best, -MH

raphaelvallat commented 3 years ago

I see! Well, I guess it makes sense to use a default value of 1 then since it is also the default in scipy.signal.periodogram and scipy.signal.welch. But I think an even better solution would be to replace the method=" argument and replace it by something like estimator=scipy.signal.periodogram or scipy.signal.welch together with **kwargs arguments that can be passed directly to the callable function. What do you think? And maybe we could use Welch as the default instead of the periodogram then?

Thanks for the input, I appreciate it! Raphael