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

ellipord does returns bogus values if gstop or gpass are negative (Trac #928) #1455

Closed
scipy-gitbot opened this issue Apr 25, 2013 · 5 comments · Fixed by #11156 or #11347
Closed

ellipord does returns bogus values if gstop or gpass are negative (Trac #928) #1455

scipy-gitbot opened this issue Apr 25, 2013 · 5 comments · Fixed by #11156 or #11347
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected good first issue Good topic for first contributor pull requests, with a relatively straightforward solution Migrated from Trac scipy.signal
Milestone

Comments

@scipy-gitbot
Copy link

Original ticket http://projects.scipy.org/scipy/ticket/928 on 2009-04-20 by @cournape, assigned to unknown.

#!python
from scipy.signal import ellipord
ellipord(0.2, 0.5, -5, -3)

Return a minimal order of 0. Potentially similar functions for other types of filters are buggy as well.

patvarilly pushed a commit to patvarilly/scipy that referenced this issue May 1, 2013
@rgommers
Copy link
Member

rgommers commented Aug 9, 2019

This now gives ValueError: cannot convert float NaN to integer, which isn't understandable. Could be easily fixed by checking the input values and raising ValueErrors with informative error messages.

Note, also for positive values if gpass > gstop:

In [26]: ellipord(0.2, 0.5, 1, 2)                                                     
Out[26]: (1, 0.2)

In [27]: ellipord(0.2, 0.5, 3, 2)                                                     
---------------------------------------------------------------------------
ValueError 

@rgommers rgommers added the good first issue Good topic for first contributor pull requests, with a relatively straightforward solution label Aug 9, 2019
@endolith
Copy link
Member

endolith commented Aug 9, 2019

And that should apply to cheb2ord, etc. too.

@rgommers
Copy link
Member

rgommers commented Jan 5, 2020

I'll reopen this because, as @endolith pointed out, there are a number of functions with the same gpass, gstop parameters that don't yet have input validation. It would be best to take the checks from gh-11156 and put them in a utility validation function, and use that in all other funcs in filter_design.py with those parameters.

@AtsushiSakai if you'd want to do that, that would be very welcome.

@rgommers rgommers reopened this Jan 5, 2020
@AtsushiSakai
Copy link
Member

@rgommers OK. I will do that : ).

@AtsushiSakai
Copy link
Member

@rgommers HI. I created a new PR. Please check it when you have time #11347

@tylerjereddy tylerjereddy added this to the 1.5.0 milestone Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected good first issue Good topic for first contributor pull requests, with a relatively straightforward solution Migrated from Trac scipy.signal
Projects
None yet
5 participants