Added digital synapses, fixed filter test #819
Conversation
|
Haha, I have the exact same change in one of my local branches, only with |
| num = num.flatten() | ||
|
|
||
| if den[0] != 1.: | ||
| raise ValueError("First element of the denominator must be 1") |
There was a problem hiding this comment.
scipy.lifter handles this by normalizing num and den by den[0] if it isn't 1. Should we adopt that instead? Personally, I spent a buttload of time trying to debug my filter because of this issue, so I'm in favour of the ValueError, just wanted to raise it as another possibility.
There was a problem hiding this comment.
I would just keep the ValueError, because it's nice and explicit if users have to normalize it themselves. Any filters designed using the Scipy functions should already have this property, anyway.
|
LGTM! Just curious about your thoughts on the inline comment. |
tbekolay
force-pushed
the
digital-synapses
branch
from
11ad897
to
bfed8de
Compare
|
Looks good! |
Added ability to specify linear synapses with digital coefficients directly, since it can often be easier to design filters directly in the digital domain. Fixed the test of general linear filters to use much more stable (less extreme) coefficients, so that backends with more limited precision can still pass the test.
tbekolay
force-pushed
the
digital-synapses
branch
from
bfed8de
to
f6318cf
Compare
Added ability to specify linear synapses with digital coefficients
directly, since it can often be easier to design filters directly
in the digital domain.
Fixed the test of general linear filters to use much more stable
(less extreme) coefficients, so that backends with more limited
precision can still pass the test.