mathnet / mathnet-numerics

Math.NET Numerics
http://numerics.mathdotnet.com
MIT License
3.51k stars 899 forks source link

Hamming and HammingPeriodic constants #641

Open Andi11449 opened 5 years ago

Andi11449 commented 5 years ago

while replacing the Hamming function used in an existing project with the MathNet version in the Window class I noticed that the constant values are different to the ones used by Matlab and Python leading to failing unit tests. Is there a reason why a, b are defined as 0.53836 and 0.46164 instead of 0.54 and 0.46 (which seems to be the implementation in most libraries)? The used values are even farther away from 25/46 and 21/46 which seems to be the most common interpretation of the window. I would suggest to change them to 0.54 and 0.46 to be consistent with other common libraries like Matlab and Python.

cdrnet commented 5 years ago

It seems there are three common choices for these constants. To my understanding, our coefficients minimize peak side-lobe levels, but it indeed deviates slightly from the standard definition. In similar cases in the past we've let the user decide which definition is needed (e.g. quantiles where there are a dozen of definitions).

It seems we could provide:

Any thoughts on that?

Andi11449 commented 5 years ago

Sounds like a good solution, it could be provided as optional argument to choose from with Equiripple Hamming as default value to not change any current usage of the function. Should I go ahead and put that in a new fork?