-
Notifications
You must be signed in to change notification settings - Fork 11
Filtfilt #86
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
Open
SpookyYomo
wants to merge
70
commits into
qsib-cbie:main
Choose a base branch
from
SpookyYomo:filtfilt
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Filtfilt #86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The error variants here should guide the user as to the wrong use of functions, instead of taking down the entire program with a panic, which is especially necessary in an embedded environment.
This however does not specify to the end user which arguments are raising the error. Fix "Conflict" typo in Error enum variant
ndarray_conv provides linear and FFT convolution for N-dimensional tensors. This commit thus introduces into core under a "num_rs" namespace, since there are many scipy functions that use numpy functions. The ConvolveMode enum is thus moved, and the relevant enum variants of ndarray_conv is also provisioned by means of `.into()`.
Display trait is for when `main` ends and an error message is shown to the end user.
There will be other functions in sci-rs::signal space that uses the linear convolution.
One can use into the Make and ArrayView from Array but not the other way round, this makes the function signature abit more friendly without needing to use `.to_owned()` needlessly.
There is nothing distinguishing between kernel and signal with both arguments now being identical in type.
This is to ensure both branches are tracking Cargo.toml in sci-rs-core
Unfortunately, it's not quite possible to specify at compile time that `axis: isize` sastisifes the bounds wrt `N`. Also check that a[0] exists and is non-zero.
Variable lifetime capture through reborrow only for subsequent `assign_to` is unnecessary.
This is later checked in `_validate_x()` function. We instead frontload it into the body.
This version of ndarray-conv accounts for both cross-correlation and convolution.
This updates ndarray-conv to 0.5.0.
Behavior of convolve in ndarray-conv was changed with 0.5.0.
This is a squash of 3 commits: 1. Failing unit test with a panic atm, more work needs to be done. 2. Further lfilter fix 3. Fix lfilter for 1D signal Had to step into python to actually see what the slicing was doing...
`alloc` feature flag for lfilter is already specified in the parent module.
Progressively make the test cases more complex so that its easier to identify. We also clarify the filter as being FIR as the `a` polynomial has been set to 1.
This helps reduce from the flow of lfilter code.
Found within .cc file. Only provide the function signature first. This function is called when `a.len() > 1`. Function signature of lfilter is also changed to reflect what zi is expected to be from both lfilter and linear_filter.
Made it clear that it was taking the shape instead of the ndim of a NDArray and making into a array `[T; N]`.
Instead of refactoring the non-zi code to follow the structure of zi code, which results in an additional heap allocation, we instead branch the code so that the scenario where zi is None does not need to undergo the additional heap allocation step.
If any error occurs in numpy-esque convolve, we can propagate it out instead of unwrapping it and panicking.
ndarray::SliceInfo::try_from only works for [SliceInfoElem; N], where N is up to and including 8. As such, whiltout resorting to Nightly compiler for `generic_const_exprs` feature, we will eventually implement lfilter for Array<_, Dim<[Ix; N]>> instead. Due to some other requirements, lfilter will instead be implemented up to 6 dimensions. See rust-lang:rust#76560.
Correspondingly update all test cases. See previous commit for rationale.
Currently, the as_strided trick is still not quite working. This commit also adds a corresponding unit test.
ShapeBuilder trait along with Array::as_slice allows as to replicate the stride_tricks method to create a different array view. It is not yet presently clear if as_slice_memory_order should be used instead, but the tests that are included are passing. It might be possible to remove the reallocation of zi at the start of the scope now that this is settled.
Implements even_ext under impl of FiltFiltPadType for IxN arrays.
Implements const_ext under impl of FiltFiltPadType for IxN arrays.
Short circuit to return an owned variant if n = 0 for `*_ext`.
Also added the corresponding test cases.
Rather than panicking, an error can be returned from the caller.
The `'a` lifetime can be elided.
This is used in filtfilt.
Obtained from python.
This avoids the use of RawData and makes return type T more explicit with container S.
The shape as `[usize; N]` function is not used by just lfilter. Also changed the name and documentation to reflect the `const N` requirement.
There is no requirement that the arguments passed into lfilter has to be IIR in the way where the denominator length is identical to the numerator length. FIR can be treated as a special case of IIR as a matter of fact.
The functionality currently works, but the signature cannot explicitly specify the return type to be `D`. This is made possible by using heap allocation of the `SliceInfo` through `Vec` instead of using the macro/trait method in Lfilter, in an attempt have another method to circumvent the need for a nightly compiler that uses similar trait bounds as NDArray without needing a nightly compiler. This function is eventually used in even/odd/const extensions (which has only been implemented on nalgebra arrays and not NDArrays) so far.
This are thin wrappers around axis_slice_unsafe with and without checks.
Previously, it was assumed that the only valid values of start and end must be contained within `0 <= idx < x.shape()[axis]` after unwrapping the negative and counting from the back (starting with -1), hence the use of a max clamp. However, this was wrong, as can be validated against scipy internal. Thus, a case for this "more negative" than `axis_len` is spelt out, with the appropriate fix (removing clamp to 0) is done.
Also specify that filtfilt_gust is a separate method that is not yet implemented. As this strongly depends on `lfilter` as a function, with `lfilter` currently only supported for FIR, it does not yet work on IIR.
The values were generated with python.
This is still valid only for FIR as lfilter is designed specifically for FIR windows so far.
Similar to lfilter, using macros to help generalize to multiple dimensions. Correspondingly add a test case for 1-dim input.
Assume init for uninit arrays instead of specifying zeros with required shape gives an improved speed in lfilter.
Tried to see in Cargo-asm if this is inlined but `cargo-show-asm` isn't picking up `lfilter` as a function. This will have to be tested in the benching function.
This centralizes areas of concern.
Callee should have the responsibility of constructing the `Axis` object.
These are fairly generic functions that parse user input into valid values of usizes (in their respective context being the arrays they act on), and thus are more suited to be in array tools.
Some of the trait requirements bound to lfilter as a function can be removed.
This is necessary for the return type to be consistent with lfilter.
d052c74 to
53b7e8e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements filtfilt on ndarrays for
b, afrom FIR filters.This FIR filter requirement is a consequence of the state of implementation from #78.
Blockers:
lfilter, asfiltfiltutilises the result.