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

MAINT: signal: adjust digital filter design warnings #11030

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

e-q
Copy link
Contributor

@e-q e-q commented Nov 8, 2019

tl,dr: The logic for raising BadCoefficients warnings needs improvement.

As mentioned in #7345, innocuous usage of digital filters can result in BadCoefficients warnings being thrown, leading to unwarranted distrust in the results. This comes down to signal.normalize throwing this warning when encountering for small elements on the leading side of the numerator (atol=1e-14), before throwing them away. I haven't yet found references or other libraries that recommend or implement this step when designing digital filters.

This isn't the best behavior for digital filters, there are reasonable cases where you end up having zeros or small coefficients at the leading edge of the numerator, such as adding a delay to an FIR filter or using many of the resultant filters that come out of firwin.

In fact, the normalize docstring states that the function is only intended for continuous (s-plane) systems, but we've been using it for digital (z-plane) systems as well. Another sign that this situation isn't ideal is the fact that many of our own unit tests filter away this error.

#1178 shows the original motivation for the BadCoefficents warning (I think), but the real issue is asking for too high order of a TF form filter. This issue has cropped up fairly often, which is why we recommend using second order sections.

This PR is currently incomplete. So far, I've removed the current check for small leading elements in the filter numerator and made the necessary adjustments to the docs and tests.

What remains to be done is to determine and implement the cases in which BadCeofficients will be raised. This could, for instance, be done at calls to IIR filter design functions that request TF output and high filter orders. If our own functions are the ones providing the coefficients in the first place, then we should be warning the user sooner if they're no good. Similarly, if the user has their own coefficients that they want to use, we should be careful about throwing values away.

Some research needs to be done to figure out a heuristic that makes sense, maybe by testing zpk->tf->zpk round trips and seeing how much the zeros and poles move...

Closes #7345

@lucascolley lucascolley added the maintenance Items related to regular maintenance tasks label Dec 23, 2023
@lucascolley lucascolley marked this pull request as draft March 14, 2024 21:44
@lucascolley lucascolley changed the title WIP: Adjust digital filter design warnings MAINT: signal: adjust digital filter design warnings Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks scipy.signal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

signal.normalize's BadCoefficients warning is too scary
2 participants