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.

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

Wrong results when cascading biquads in sosfilter_c #24

Open
happyTonakai opened this issue Jan 6, 2023 · 10 comments
Open

Wrong results when cascading biquads in sosfilter_c #24

happyTonakai opened this issue Jan 6, 2023 · 10 comments

Comments

@happyTonakai
Copy link

happyTonakai commented Jan 6, 2023 •
edited
Loading

sosfilter_c gives wrong results when cascading multiple biquads. Some simple code to show:

import numpy as np
from pyfilterbank.sosfiltering import sosfilter_c
from scipy.signal import sosfilt

sos0 = np.array([
    [0.9994671940803528, -1.980954885482788, 0.9816480278968811, 1.0, -1.9809452295303345, 0.981124997138977],
    [0.9940603375434875, -1.8794806003570557, 0.89670330286026, 1.0, -1.8794806003570557, 0.8907635807991028]
])
sos1 = sos0.flatten()

x = np.random.random((16,))
z0 = np.zeros((sos0.shape[0], 2))
y0, z0 = sosfilt(sos0, x, zi=z0)

y1, z1 = sosfilter_c(x, sos1)

y2, _ = sosfilter_c(x, sos0[0])
y2, _ = sosfilter_c(y2, sos0[1])

y0 and y2 are identical except the numerical precision but y1 is incorrect.
@SiggiGue
Copy link
Owner

SiggiGue commented Jan 11, 2023 via email

@happyTonakai
Copy link
Author

@SiggiGue Thank you for response. Could you please specify where the wrong usage is?

I've read the documentation and the syntax of the function is

signal_out, states = sosfilter_c(signal_in, sos, states=None)

signal_in has a shape of (N, 0) and sos has a shape of (K*6, 0). states can be None.

In my code x has a shape of (16, ), sos1 has a shape of (2*6, ), and initial states is None. These usage are all met in my example code and I don't find anything goes wrong. I also tried to convert sos and x into np.float32 which doesn't work as well.

By the way, I think there's something wrong in your unit test code:

pyfilterbank/pyfilterbank/tests/test_sosfiltering.py

Lines 10 to 18 in 5d74af8

def setUp(self):
self.signal = np.random.randn(1000)
self.frames = len(self.signal)
b = [0.5, 0.0, 0.5]
a = [1.0, -0.8, 0.7]
sos = [b + a]
sos = np.array(sos + sos).astype(np.float32)
self.sos = sos
self.ksos = int(len(sos)/5)

In the test code sos has a shape of (2, 6) and results in ksos=0 because len(sos)=2.

@happyTonakai
Copy link
Author

You can check this out by replacing sf.sosfilter_double_c with sf.sosfilter_c in the ut

pyfilterbank/pyfilterbank/tests/test_sosfiltering.py

Line 23 in 87b9bb5

oc, sc = sf.sosfilter_double_c(self.signal.copy(), self.sos)

and you will see the unit test failed:

Traceback (most recent call last):
  File "~/ut.py", line 41, in test_implementation
    self.assertAlmostEqual(sop, soc, places=6)
AssertionError: 1119.841385113597 != 1121.028 within 6 places (1.1865689879655292 difference)

@SiggiGue
Copy link
Owner

SiggiGue commented Jan 12, 2023 via email

@happyTonakai
Copy link
Author

If you cascade sos, you need too feed the intermediate states into the function as well to get the expected results for your comparison.

This doesn't make sense to me. The documentation states that initial state can be None, and even I change this line

y1, z1 = sosfilter_c(x, sos1)

to

y1, z1 = sosfilter_c(x, sos1, states=np.zeros((2 * 2, )))

the result is still incorrect. If you meant that

y2, _ = sosfilter_c(x, sos0[0])
y2, _ = sosfilter_c(y2, sos0[1])

should be

y2, state = sosfilter_c(x, sos0[0])
y2, _ = sosfilter_c(y2, sos0[1], state)

I don't think this is true. Cascaded biquads don't share internal states. y2 gives the same result as scipy.signal.sosfilt.

@happyTonakai
Copy link
Author

Have a look at this code:

import numpy as np
from pyfilterbank.sosfiltering import sosfilter_c, sosfilter_double_c
from scipy.signal import sosfilt

sos = np.array([
    [0.9994671940803528, -1.980954885482788, 0.9816480278968811, 1.0, -1.9809452295303345, 0.981124997138977],
    [0.9940603375434875, -1.8794806003570557, 0.89670330286026, 1.0, -1.8794806003570557, 0.8907635807991028]
], dtype=np.float32)

x = np.random.random((16,)).astype(np.float32)

y0, z0 = sosfilt(sos, x, zi=np.zeros((sos.shape[0], 2)))
y1, z1 = sosfilter_c(x, sos.flatten(), states=np.zeros((sos.shape[0] * 2, )))
y2, z2 = sosfilter_double_c(x, sos.flatten(), states=np.zeros((sos.shape[0] * 2, )))

print(np.mean((y0 - y1)**2))
print(np.mean((y0 - y2)**2))

The only difference between y1 and y2 is sosfilter_c and sosfilter_double_c, and the output is quite different.

0.0013462692831356475
3.282670120428347e-29

@SiggiGue
Copy link
Owner

SiggiGue commented Jan 12, 2023 via email

@SiggiGue SiggiGue reopened this Jan 12, 2023
@SiggiGue
Copy link
Owner

SiggiGue commented Jan 12, 2023 via email

@happyTonakai
Copy link
Author

image

As you can see the output is so different that we can't say it's related to the numeric issue. The states of the first stage of sos is correct but the second one is not. I've checked the source code and all I found is

pyfilterbank/pyfilterbank/sosfiltering.py

Lines 109 to 110 in 87b9bb5

if isinstance(states, type(None)):
states = np.zeros(ksos*2).astype(np.double)

can be changed to

 if isinstance(states, type(None)): 
     states = np.zeros(ksos*2).astype(np.float32)

but this doesn't help because you convert it to np.float32 in the following lines

pyfilterbank/pyfilterbank/sosfiltering.py

Lines 112 to 113 in 87b9bb5

states_c = ffi.new(
'char[]', np.array(states, dtype=np.float32).flatten().tostring())

C code seems correct too. I can't tell what's going wrong.

@SiggiGue
Copy link
Owner

SiggiGue commented Jan 12, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@SiggiGue @happyTonakai