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

Spectral entropy not stable #10

Open
MaxHeuillet opened this issue Oct 29, 2020 · 3 comments
Open

Spectral entropy not stable #10

MaxHeuillet opened this issue Oct 29, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@MaxHeuillet
Copy link

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 raphaelvallat self-assigned this Oct 29, 2020
@raphaelvallat raphaelvallat added the enhancement New feature or request label Oct 29, 2020
@raphaelvallat
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants