Question in the title. Is convolving a matrix with a 3x3,5x5,7x7 etc kernel something which you would consider useful for this library or is it already too abstract?
Oh great! The documentation links dont seem to be working for this one. Im assuming the matrix has to be column major as well.
Mmh, yeah, the documentation seems broken there.
Yous the matrix has to be column-major (all matrices on nalgebra are column-major).
Im struggling to use this functionality. Ideally I want to use this with DMatrix types. But so far, even this snippet wont compile:
use na::base::{Vector,Vector6};
use na::linalg;
fn main() {
let vec = Vector6::new(1,2,3,4,5,6);
let filter = Vector1::new(1);
vec.convolve_full(filter);
}
I get the following error message:
no method named `convolve_full` found for type `na::Matrix<{integer} ..,
From the source I can tell its defined for type Vector. But how can I access the underlying Vector type?
This is actually no “underlying Vector type” since vectors are also represented as na::Matrix
. The issue here is that you are using integer matrices instead of floats. It appears the current implementation only exists for floating-point matrices. See the N: RealField
bound there: https://github.com/rustsim/nalgebra/blob/dev/src/linalg/convolution.rs#L9
I think this restriction is too strong and the same code should work by replacing RealField
by Scalar + Field
(where Field
comes from alga::general::Field
). Would you like to contribute make a PR with this change if it happens to actually work?
Sure I can take a look, but it does not work for Matrices.
So the Vector example now compiles
let vec = Vector6::new(1.0,2.0,3.0,4.0,5.0,6.0);
let filter = Vector1::new(1.0);
vec.convolve_full(filter);
However, this does not compile
let mat = Matrix3::new(1.0,0.0,0.0,0.0,1.0,0.0,0.0,0.0,1.0);
let filter_mat = Matrix1::new(1.0);
mat.convolve_full(filter_mat);
The error is:
error[E0599]: no method named `convolve_full` found for type `na::Matrix<{float}, na::U3, na::U3, na::ArrayStorage<{float}, na::U3, na::U3>>` in the current scope
Sure I can take a look
Great, thanks!
but it does not work for Matrices.
Ah, yes. The current implementation only supports float numbers of matrices with only one column (which Vector
are just a type alias for). So there are actually two independent possible improvements:
- Generalize the existing code so it works on integers. This should be easy as stated in my previous comment.
- Generalize the existing code so it works on matrices (with more than one column). This will probably be more difficult to implement.
Great. i thought I might have missed something. Ill take a look at both tasks. Should be fun.
There seems to be and issue with Scalar + Field
= note: the method `convolve_same` exists but the following trait bounds were not satisfied:
`{integer} : alga::general::Field`
error[E0277]: the trait bound `{integer}: approx::RelativeEq` is not satisfied
I dont really want to change any type definitions.
That may be because the compiler assumes your inputs are unsigned integers, for which Field
is not implemented. Let’s try even more permissive traits: N: Scalar + ClosedMul<N> + ClosedAdd<N> + Zero
. I think this is all the operations we rely one here. The ClosedMul
and ClosedAdd
traits come from alga::general::
too.
I cant find a Zero trait. I found Identity but that does not seem to work.
The Zero
trait comes from the num
crate. So it comes from use num::Zero
.
Convolution works of Integers and Floats now.
I also had to add the
Identity<Additive>
trait for zero::<N>()
I made a PR for all the convolution things