Skip to content

Updated the buttord function to be consistent with the other filter #3235

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Updated the buttord function to be consistent with the other filter #3235

wants to merge 4 commits into from
+20 −20

Conversation

cimarronm
Copy link
Contributor

design methods and optimize the order and natural frequencies such that
the passband is just met while maximizing the stopband (or vice versa
for stopband filter).

@cimarronm cimarronm mentioned this pull request Jan 23, 2014
Closed
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c61982f on cimarronm:buttord_design_tweak into 635b2d2 on scipy:master.

endolith added a commit to endolith/scipy that referenced this pull request Jan 29, 2014
@endolith
TST: Check frequency response for filter design tools
4afdc53 
The *ord functions design a filter to match a specification, so we should check whether the resulting frequency response actually meets the specification.

Also change the tests for functions that don't match matlab, considering their current behavior to be correct.  If scipy#3235 changes their behavior, the tests will be changed to match.
@rgommers rgommers mentioned this pull request Jan 31, 2014
Merged
@rgommers
Copy link
Member

@cimarronm this seems to have a merge conflict (already did before merging gh-3085), could you rebase your branch? And can you also update the test for this function that was added by @endolith in gh-3085?

@rgommers
Copy link
Member

Added the 0.14.0 milestone - I'd like to get this in for the same release as gh-3085.

@cimarronm
Updated the buttord function to be consistent with the other filter
ca95668 
design methods and optimize the order and natural frequencies such that
the passband is just met while maximizing the stopband (or vice versa
for stopband filter).
@cimarronm
Updated butterworth filter tests for update to buttord calculation. A…
71cdcde 
…lso tightened the passband/stopband checks.
@rgommers
Copy link
Member

rgommers commented Feb 1, 2014

TravisCI failures are real:

======================================================================
FAIL: test_bandpass (test_filter_design.TestButtord)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/scipy/scipy/build/testenv/lib/python2.6/site-packages/scipy/signal/tests/test_filter_design.py", line 267, in test_bandpass
dB(h[np.logical_and(wp[0] <= w, w <= wp[1])]))
File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/testing/utils.py", line 880, in assert_array_less
header='Arrays are not less-ordered')
File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/testing/utils.py", line 644, in assert_array_compare
raise AssertionError(msg)
AssertionError:
Arrays are not less-ordered
(mismatch 0.649350649351%)
x: array(-3)
y: array([ -2.14653787e+00, -1.15393961e+00, -5.85080766e-01,
-2.85614174e-01, -1.36155875e-01, -6.34113274e-02,
-2.91832712e-02, -1.30302629e-02, -5.36565872e-03,...

======================================================================
FAIL: test_bandstop (test_filter_design.TestButtord)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/scipy/scipy/build/testenv/lib/python2.6/site-packages/scipy/signal/tests/test_filter_design.py", line 291, in test_bandstop
rtol=1e-15)
File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/testing/utils.py", line 1181, in assert_allclose
verbose=verbose, header=header)
File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/testing/utils.py", line 644, in assert_array_compare
raise AssertionError(msg)
AssertionError:
Not equal to tolerance rtol=1e-15, atol=0
(mismatch 100.0%)
x: array([ 0.1475943 , 0.59997366])
y: array([ 0.14759432, 0.59997366])

The second one should be a matter of adding more decimals to the expected result and/or lowering rtol. The first one needs investigating.

@cimarronm
Copy link
Contributor Author

I tightened the constraints on the checkers which is why these two are failing; I can revert back to what it was before and they should pass I believe. The strange thing is that these tests don't fail for me on two different systems I have so something is different between the travis system and mine.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling b0e4771 on cimarronm:buttord_design_tweak into b52aba6 on scipy:master.

@rgommers
Copy link
Member

rgommers commented Feb 2, 2014

The rp + 0.1 one was some numerical noise issue that @endolith brought up in another thread. IIRC there was no good function to do assert_array_less with some tolerance.

For the other test, I'd put the number at 1e-6 since that still works for Travis.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0a8bf80 on cimarronm:buttord_design_tweak into b52aba6 on scipy:master.

rgommers pushed a commit that referenced this pull request Feb 3, 2014
Merge branch 'pr-3235' into master.
f3acfe3 
Reviewed at #3235
@rgommers
Copy link
Member

rgommers commented Feb 3, 2014

Thanks @cimarronm. Squashed the last few commits, edited the commit messages a bit and pushed this PR in f3acfe3.

@rgommers rgommers closed this Feb 3, 2014
@endolith endolith mentioned this pull request May 5, 2018
Merged
@endolith endolith mentioned this pull request Jul 23, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.signal
Projects
None yet
Milestone
0.14.0
Development

Successfully merging this pull request may close these issues.
3 participants
@cimarronm @coveralls @rgommers