NWaves icon indicating copy to clipboard operation
NWaves copied to clipboard

Buggy RLS filter implementation

Open richardpl opened this issue 1 year ago • 3 comments

This line looks wrong:

https://github.com/ar1st0crat/NWaves/blob/c1a6a4a8fdb70ce7cfd6b31ad59595e8461ff2e5/NWaves/Filters/Adaptive/RlsFilter.cs#L112

Are there any tests to make sure that this implementation behave correctly?

richardpl avatar Apr 21 '23 17:04 richardpl

Looks like an ordinary matrix multiplication (straightforward implementation, not the most efficient, though). I didn't write tests for adaptive filters, but there are demo forms that allow us checking the results. In particular, this form:

image

ar1st0crat avatar Apr 22 '23 18:04 ar1st0crat

Even if implementation idea is correct, the matrix multiplication I linked above is flawed. One needs +=. Otherwise older values are overwritten.

richardpl avatar Apr 22 '23 19:04 richardpl

You're right - it's a bug; it should be +=. Thanks for noticing. I'll double-check this and fix it when I have the time

ar1st0crat avatar Apr 22 '23 19:04 ar1st0crat