Does Filtering / Convolution fit into this library?

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?

Hi! We already have a simple implementation of convolution there.

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:

  1. Generalize the existing code so it works on integers. This should be easy as stated in my previous comment.
  2. 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 ClosedAddtraits come from alga::general:: too.

I cant find a Zero trait. :confused: 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.

1 Like

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