Compilation error with CsMat and generic Float

[relf]

I am new to sprs library, I do not understand why the following program does not compile:

use num_traits::Float;
use sprs::CsMat;

fn test<F: Float>(val: F) -> CsMat<F> {
    let a = CsMat::new_csc(
        (3, 3),
        vec![0, 2, 4, 5],
        vec![0, 1, 0, 2, 2],
        vec![val, val, val, val, val],
    );
    let b = &a + &a;
    b
}

fn main() {
    let a = test(1.);
}

with the following dependencies

[dependencies]
sprs = "0.10"
num-traits = "0.2"

and when I try to build I get:

error[E0369]: cannot add `&CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>` to `&CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>`
  --> src\main.rs:11:16
   |
11 |     let b = &a + &a;
   |             -- ^ -- &CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>
   |             |
   |             &CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>

error: aborting due to previous error

For more information about this error, try `rustc --explain E0369`.
error: could not compile `test-sprs`

To learn more, run the command again with --verbose.

I cannot see what is missing to enable the addition. Thanks for your help.

[mulimoen]

In short: F must support addition, and the error message is poor.

Longer: To support addition for arbitrary types, we must restrict F by applying the appropriate trait bounds. Which ones are a bit cryptic, but we can dig through the sprs code to find the impl for Add here: https://github.com/vbarrielle/sprs/blob/f815271c8543981d6c926fe2415ef722bda7ceaa/src/sparse/binop.rs#L20 The bound on F is given by the following line: https://github.com/vbarrielle/sprs/blob/f815271c8543981d6c926fe2415ef722bda7ceaa/src/sparse/binop.rs#L24 You can modify test to include this bound by:

fn test<F: Float>(val: F) -> CsMat<F>
where
    F: num_traits::Zero + PartialEq + Clone + Default,
{

and your code should compile.

Edit: It should be enough to specify the additional trait bound Default.

[relf]

Thanks for the swift reply. Actually I went down that way, but in that case I got:

error[E0308]: mismatched types
  --> src\main.rs:14:18
   |
14 |     let b = &a + &a;
   |                  ^^ expected struct `ndarray::ArrayBase`, found struct `CsMatBase`
   |
   = note: expected reference `&ndarray::ArrayBase<_, ndarray::dimension::dim::Dim<[usize; 2]>>`
              found reference `&CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>`

error[E0308]: mismatched types
  --> src\main.rs:15:5
   |
4  | fn test<F: Float>(val: F) -> CsMat<F>
   |                              -------- expected `CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>` because of return type
...
15 |     b
   |     ^ expected struct `CsMatBase`, found struct `ndarray::ArrayBase`
   |
   = note: expected struct `CsMatBase<F, usize, Vec<usize>, Vec<usize>, Vec<F>>`
              found struct `ndarray::ArrayBase<ndarray::data_repr::OwnedRepr<F>, ndarray::dimension::dim::Dim<[usize; 2]>>`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
error: could not compile `test-sprs`

It seems to pick the addition with a ndarray! Here the modified program:

use num_traits::Float;
use sprs::CsMat;

fn test<F>(val: F) -> CsMat<F>
where
    F: Float + num_traits::Zero + PartialEq + Clone + Default,
{
    let a = CsMat::new_csc(
        (3, 3),
        vec![0, 2, 4, 5],
        vec![0, 1, 0, 2, 2],
        vec![val, val, val, val, val],
    );
    let b = &a + &a;
    b
}

fn main() {
    let a = test(1.);
}

[mulimoen]

Oh, I see my cargo add used 0.9 of sprs. That does not look good, I'll check again to see what is going on here

[mulimoen]

My next attempt involves adding this bound:

for<'r> &'r F: core::ops::Add<&'r F, Output = F>,

but this leads to a recursion error in type resolution.

[relf]

Yep did that too! :eyes: Then I opened the issue thinking I was missing something.

[mulimoen]

Paging @vbarrielle, this was introduced in d2f0da76. Any ideas in why type-checking would go haywire here?

[relf]

I confirm it compiles with 0.9.3 adding only the Default trait as you mentioned. As far as I am concerned, I want to use ndarray 0.14, so...

[vbarrielle]

I'll need to investigate this issue. I've definitely encountered the type resolution recursion issue (see https://github.com/rust-lang/rust/issues/82779) while developping d2f0da7.

This error happens because I've tried to minimize the number of clones in the binary operations, and I picked my traits to be sure that it would work on classic scalar types. However I failed to consider the case of a dependent crate using a generic scalar type, which is quite a problem...

[bytesnake]

any update here? This blocks bumping ndarray = "0.14" atm

[mulimoen]

As a temporary measure we could backport the ndarray bump to 0.9.

[relf]

Yes, I think it would be great to have that backup solution in the meantime, not to fall too much behind as ndarray 0.15.1 is already out.

[mulimoen]

@vbarrielle I've added a new branch 0.9.4 branching off the 0.9.3 tag. This bumps ndarray for sprs and sprs-ldl.

[relf]

@mulimoen FWIW, your 0.9.4 branch works for linfa with ndarray 0.14 (https://github.com/rust-ml/linfa/pull/110).

[vbarrielle]

Hello, sorry for the silence, things are a bit complicated here and I don't have time to work on this right now (and probably for the next month as well).

Thanks @mulimoen for making this 0.9.4 branch. @mulimoen if you accept I'm giving you upload rights on crates.io to push that version.

[mulimoen]

Thanks @vbarrielle, 0.9.4 should be uploaded now.

[bytesnake]

Hello, sorry for the silence, things are a bit complicated here and I don't have time to work on this right now (and probably for the next month as well).

sure take your time, it is not super urgent but we don't want to lack too far behind ndarray

Thanks @vbarrielle, 0.9.4 should be uploaded now.

cool thanks!

[relf]

Thanks @vbarrielle for your hard work!

[mulimoen]

I think the original compilation error is gone with the following extra bounds:

fn test<F>(val: F) -> CsMat<F>
where
    F: Float,
    F: Default + Send + Sync + num_traits::MulAdd<Output = F>,
{
    let a = CsMat::new_csc(
        (3, 3),
        vec![0, 2, 4, 5],
        vec![0, 1, 0, 2, 2],
        vec![val, val, val, val, val],
    );
    let b = &a * &a;
    b
}

[relf]

Yes, it works, thanks! I am wondering how you found that list of bound spells though! :smile:

I still have to try this with linfa :sweat_smile:. Thanks again.

[bytesnake]

this seems to be broken again (perhaps a regression introduced in #290). The problem can be worked around by using csmat_binop(a.view(), a.view(), |x, y| x.add(*y)) instead of this https://docs.rs/sprs/0.11.0/src/sprs/sparse/binop.rs.html#63 which aborts with type recursion. Reopen?

[mulimoen]

@bytesnake This does not seem related to the original issue. I've opened #292 to address this issue edit: This is indeed the same problem

[mulimoen]

I also see that my "fix" did not actually work as I replaced addition by multiplication

[StefanUlbrich]

Did anybody find a workaround? How does this recursion error occur?

[StefanUlbrich]

Would it help to create another blanket type around for the HRBT? That code does not raise the recursion error where it did before but I cannot really test it right now:

pub trait Real<T>: Num + NdFloat + Default {}
pub trait RealRef<S, T>: Add<S, Output=T> + Mul<S, Output=T> {}

impl Real<f32> for f32 {}
impl Real<f64> for f64 {}

impl RealRef<&f32,f32> for &f32 {}
impl RealRef<&f64,f64> for &f64 {}

fn test1<T, D>( y: Array<T, D>)
where
    T: Real<T>,
    for<'r> &'r T: RealRef<&'r T, T>,
    D: Dimension,
{
}

Could sprs provide a suitable blanket trait comparable to NDFloat if this worked?

Edit: It seems to work

[mulimoen]

@StefanUlbrich How should this fix be integrated into sprs?

[StefanUlbrich]

It should suffice to include the two traits with implementations for the relevant types (does integer or complex make sense?). That way, sprs can be used in libraries with generic types. They could be called SPRSfloat and SPRSfloatRef (unless someone more experienced with rust knows how this can be achieved with a single one). In addition, I combined only the traits that I needed for the csaps-rs package. More might make sense

edit: do you have a recommendation for a location, @mulimoen ? I’d be happy to make a PR