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

Unusual results when using Bessel filters of higher orders #25

Closed
anleu opened this issue Sep 8, 2023 · 5 comments
Closed

Unusual results when using Bessel filters of higher orders #25

anleu opened this issue Sep 8, 2023 · 5 comments

Comments

@anleu
Copy link

anleu commented Sep 8, 2023

We've observed that the results from IIRJ align closely with Scipy for most filters. However, for Bessel Filters of order greater than 1, there are significant differences. Visually, the Bessel results from IIRJ appear inconsistent. Given that other filters produce similar results to Scipy, we suspect this isn't a configuration issue, though we can't be entirely certain. Any insights or guidance on this matter would be highly appreciated.

Compare.zip

@berndporr
Copy link
Owner

berndporr commented Sep 8, 2023

Bessel filters have no analytical solution and the code relies on finding the roots of the polynomials. org.apache.commons.math3.analysis.solvers.LaguerreSolver is used for this. As you know it's based on C++ code and converted to java. The C++ code had a buggy root finder and Bessel has been removed a while ago. Looks like the root finder of apache commons has an issue as well (or the Bessel function has been buggy but worked till recently as the roots were not affected). I guess the best plan of action is to remove Bessel completely and rather import the coefficients calculated by Scipy. That's also the way to do it for C++. I certainly have no time to get to the bottom of it as it's non-trivial and takes a lot of time.

berndporr added a commit that referenced this issue Sep 8, 2023
As @anleu pointed out in #25
there is a mismatch between the output scipy generates and the output
here. Since Bessel polynomials have no analytical solution the roots
need to be found by an iterative search algorithm. Looks like that
the one by Scipy and the one by this library is different. Certainly
the results are different. This looks like either problem in the
root finder or the Bessel polynomial itself. Till that has been
properly investigated Bessel.class has been removed for safety
reasons.
@berndporr
Copy link
Owner

Of course I'd be more than happy to accept pull requests which make sure the impulse responses match. Ideally also the unit tests should receive coefficients from python for comparison.

@berndporr
Copy link
Owner

Thanks for cross checking though!

@anleu
Copy link
Author

anleu commented Sep 9, 2023

It seems that Scipy doesn't use a solver but instead has fixed results for the poles.
https://github.com/scipy/scipy/blob/f2ec91c4908f9d67b5445fbfacce7f47518b35d1/scipy/signal/filter_design.py#L2351

Perhaps this is also an approach for IIRJ?
I tried it with the given example of order 5. It looks slightly better, but there are still differences. I don't have time at the moment to investigate further. I'll try to take another look in two weeks.

Bessel Order 5 Compare

@anleu
Copy link
Author

anleu commented Oct 5, 2023

I will prepare the PR after #27 is merged.

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

No branches or pull requests

2 participants