numpy-esque linear convolve #77
Open
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
Contributor
Author
|
I will open a relevant PR in due time to utilise ndarray-conv 0.5, which should so implement the remaining convolution patterns required in #76. |
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.
SpookyYomo
force-pushed
the
np-convolve
branch
from
5fc2154 to
686d1d2
Compare
SpookyYomo
force-pushed
the
np-convolve
branch
from
686d1d2 to
182658d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
There are scipy functions such as lfilter that use numpy's linear convolve
over scipy's fft convolve explicitly. While it is possible for us to implement
ourselves, the hope is that the shared dependency from sci-rs-signal can be
shared with sci-rs-core. Also, hopefully another library can instead be the
one to provide SIMD/etc performance.
For now, this PR merely scopes the function to be feature parallel with
numpy's by utilising
ndarray-convcrate.The dependency brought into sci-rs-core naturally uses
ndarray/rayon, which mightmean that this entire feature has to be gated behind
std(or perhaps even a rayonwhich automatically enables std) feature flag.
Blockers:
Note
Introduces a new core crate with numpy-like 1D convolution and shared error types, and updates sci-rs to depend on it and reuse its ConvolveMode via feature-propagated flags.
sci-rs-core):no_std/allocfeatures and sharedErrortype.num_rs::convolveandConvolveMode(Full/Same/Valid) viandarray-conv, with unit tests.sci-rs-coreand propagatealloc/stdfeatures inCargo.toml.ConvolveModewith re-export fromsci_rs_core::num_rsinsignal/convolve.rs.Written by Cursor Bugbot for commit 182658d. This will update automatically on new commits. Configure here.