Skip to content

Fix phase offset of Linkwitz-Riley Allpass Filters and add testing function #185

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

Merged
merged 6 commits into from
Mar 8, 2019

Conversation

fietew
Copy link
Member

@fietew fietew commented Mar 4, 2019

For some filter orders (even, but not divisible by four), there was a wrong sign for LR Allpass Filters. This PR fix this and adds a testing function.

@fietew fietew requested review from hagenw and fs446 March 4, 2019 09:48
axis label corrected
@fs446
Copy link
Member

fs446 commented Mar 4, 2019

bug fix seems ok to me, just corrected the labeling of axis in the test script.

@hagenw
Copy link
Member

hagenw commented Mar 5, 2019

The test runs fine o my machine.
Is this the desired output:
Fig1
fig1
Fig 2
fig2
Fig 3
fig3

@fietew
Copy link
Member Author

fietew commented Mar 5, 2019

Is this the desired output:

It seems, there is a problem regarding the Octave output. The greens lines in Fig1 and Fig3 (upper left) should be horizontal. I will have a look.

@fietew
Copy link
Member Author

fietew commented Mar 5, 2019

The behaviour is, again, related to a bug in zp2sos in Octave (see https://savannah.gnu.org/bugs/?51936), where single or non-existing zeros are handled incorrect. This was already mentioned in #169 . I used a similar workaround for this.

@hagenw
Copy link
Member

hagenw commented Mar 5, 2019

Looks good now, here are the new figures I'm getting by running the test:
Fig 1
fig1
Fig 2
fig2
Fig 3
fig3

Copy link
Member

@fs446 fs446 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@hagenw
Copy link
Member

hagenw commented Mar 5, 2019

Is it ok to squash the commits during merge?

Alternatively I would propose to condense them to two, one for the fix and one for adding testing functions.

@fietew fietew merged commit a385036 into master Mar 8, 2019
@fietew fietew deleted the fix-linkwitz-riley branch October 14, 2019 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants