dimforge / nalgebra

Linear algebra library for Rust.
https://nalgebra.org
Apache License 2.0
4.05k stars 485 forks source link

Problem in assert_view_index function of matrix_view.rs? #1436

Closed JEMH closed 2 months ago

JEMH commented 2 months ago

Using algebra 0.33.0 on my MacBook, I get out of bound errors when using view_with_steps_mut. It looks to me that there is a problem in the assert_view_index function. See below for a small example that generates the error.

Is there a problem or have I done something wrong?


use nalgebra::DMatrix;

let mut matrix = DMatrix::from_row_slice(4, 4, &[
    1., 2., 3., 4.,
    5., 6., 7., 8.,
    9., 10., 11., 12.,
    13., 14., 15., 16.,
]);
let start = (1,1);
let shape = (2,2);
let strides = (2, 2);
let mut sub_matrix = matrix.view_with_steps_mut(start, shape, strides);
sub_matrix.fill(99.0);
println!("Updated matrix:\n{}", matrix);

// Expected result.
//   1   2   3   4
//   5  99   7  99
//   9  10  11  12
//  13  99  15  99

Actual result.

thread 'main' panicked at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nalgebra-0.33.0/src/base/matrix_view.rs:291:9:
    Matrix slicing out of bounds.
    stack backtrace:
0: std::panicking::begin_panic
at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/std/src/panicking.rs:693:12
1: nalgebra::base::matrix_view::<impl nalgebra::base::matrix::Matrix<T,R,C,S>>::assert_view_index
at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nalgebra-0.33.0/src/base/matrix_view.rs:291:9
2: nalgebra::base::matrix_view::<impl nalgebra::base::matrix::Matrix<T,R,C,S>>::generic_view_with_steps_mut
at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nalgebra-0.33.0/src/base/matrix_view.rs:648:13
3: nalgebra::base::matrix_view::<impl nalgebra::base::matrix::Matrix<T,R,C,S>>::view_with_steps_mut
at /Users/admin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nalgebra-0.33.0/src/base/matrix_view.rs:551:13
4: expt_vec::main
at ./expt_vec.rs:64:26
5: core::ops::function::FnOnce::call_once
at /rustc/3f5fd8dd41153bc5fdca9427e9e05be2c767ba23/library/core/src/ops/function.rs:250:5
JEMH commented 2 months ago

To me the current asserts look wrong: assert!( start.0 + (steps.0 + 1) shape.0 <= my_shape.0 + steps.0, "Matrix slicing out of bounds." ); assert!( start.1 + (steps.1 + 1) shape.1 <= my_shape.1 + steps.1, "Matrix slicing out of bounds." ); }

From my understanding I would expect the first assertion to be start.0 + (shape.0 - 1) steps.0 <= my_shape.0 - 1 which can be written without subtraction as start.0 + shape.0 steps.0 + 1 <= my_shape.0 + steps.0

Similarly I think the second assertion should be start.1 + shape.1 * steps.1 + 1 <= my_shape.1 + steps.1

Is this correct?

JEMH commented 2 months ago

After reading the documentation more closely, I see that I have misinterpreted the steps variables. the steps refer to the number of ignored rows/columns rather than the number of steps between the desired rows/columns. In my example above, I should have used (1, 1) for steps to achieve the desired result. Sorry for any inconvenience caused.