Merging firwin and firwin2 in scipy.signal

Hi, I was looking at the detailed SciPy roadmap, and one of the improvements in scipy.signal is "merging firwin and firwin2 so firwin2 can be removed." If there is still interest in this, I can work on it. Thanks, Shashaank

On 6/16/20, Shashaank N <shashaank.n@columbia.edu> wrote:
Hi,
I was looking at the detailed SciPy roadmap, and one of the improvements in scipy.signal is "merging firwin and firwin2 so firwin2 can be removed." If there is still interest in this, I can work on it.
Yes, there is still interest! Before working on any code, we need to agree on the API changes that will be needed to make this happen. The updated API needs to provide (roughly) equivalent functionality as that currently provided by firwin and firwin2. We also need to maintain backwards compatibility. I haven't looked at these in a long time--maybe the API changes will be simple, and we can start looking at implementations pretty quickly. If anyone already has ideas on this, let us know. Warren
Thanks, Shashaank

Right now, the main difference between the two functions is in how the coefficients are computed: firwin multiplies the sinc function on the cutoff frequency bands, while firwin2 linearly interpolates the cutoff frequencies with a uniform mesh. If we can define a flag that switches between the two calculation methods, then we can preserve the functionality of both. I took a similar approach recently to switch between FIR and IIR filter coefficients when implementing gammatone filters for SciPy. Parameter-wise, I think we can keep the gain, nfreqs, and antisymmetric parameters from firwin2, and remove the scale parameter from firwin. However, we might have to internally match the coefficients' order of magnitude inside the function, as firwin returns values in the 10e-4 range with scale set to True, while firwin2 returns values in the 10e-2 range. Both functions have the nyquist frequency parameter marked as "deprecated" in the docstring, but they don't throw a warning in the code. Can we add this warning to the function? - Shashaank On Fri, Jun 19, 2020 at 11:09 AM Warren Weckesser < warren.weckesser@gmail.com> wrote:
On 6/16/20, Shashaank N <shashaank.n@columbia.edu> wrote:
Hi,
I was looking at the detailed SciPy roadmap, and one of the improvements in scipy.signal is "merging firwin and firwin2 so firwin2 can be removed." If there is still interest in this, I can work on it.
Yes, there is still interest! Before working on any code, we need to agree on the API changes that will be needed to make this happen. The updated API needs to provide (roughly) equivalent functionality as that currently provided by firwin and firwin2. We also need to maintain backwards compatibility. I haven't looked at these in a long time--maybe the API changes will be simple, and we can start looking at implementations pretty quickly.
If anyone already has ideas on this, let us know.
Warren
Thanks, Shashaank
participants (2)
-
Shashaank N
-
Warren Weckesser