dimforge / dimforge/nalgebra

nalgebra::base::Matrix::swap_rows and similar methods require T to implement nalgebra::base::Scalar

Open
#1,121 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
4.8k
Forks
565
PR merge metrics
No merged PRs in 30d

Description

Hi,

I find it weird that [`nalgebra::base::Matrix::swap_rows`](https://docs.rs/nalgebra/latest/nalgebra/base/struct.Matrix.html#method.swap_rows) and similar methods require the type `T` of their entries to be a [`Scalar`]. Assume that in a downstream crate there's a [`Matrix`] with entries of a type from another crate. For the downstream crate, both the type of entries and the [`Scalar`] trait belong to external crates, which is inconvenient because it's impossible to implement external trait on external type. Of course, it's possible to work around this problem by making a local type-wrapper around the type of entries or define a local trait that would mirror [`Scalar`]. However, It's not clear for me why the bound is needed.

Consider the following implementation:

```rust
pub struct MatrixReprOfLinEq(pub Matrix);

impl MatrixReprOfLinEq
where
R: Dim,
C: Dim,
S: RawStorageMut
{
/// Switches rows `i_1` and `i_2` in the matrix.
///
/// Unlike [`nalgebra::base::Matrix::swap_rows`], this method doesn't require the entries
/// to implement [`nalgebra::base::Scalar`].
///
/// # Safety
///
/// This function is unsafe because it does not check if the indices are valid.
pub unsafe fn row_xchg(&mut self, i_1: usize, i_2: usize)
{
let ncols = self.0.ncols();
(0..ncols)
.map(|j| ((i_1, j), (i_2, j)))
.for_each(|(row_col1, row_col2)| {
self.0.swap_unchecked(row_col1, row_col2);
}
);
}
}
```

It can work even with non-`'static` types, such as arbitrary-size integers, rationals, etc.

[`nalgebra::base::Matrix::swap_rows`]: https://docs.rs/nalgebra/latest/nalgebra/base/struct.Matrix.html#method.swap_rows
[`Scalar`]: https://docs.rs/nalgebra/latest/nalgebra/base/trait.Scalar.html
[`Matrix`]: https://docs.rs/nalgebra/latest/nalgebra/base/struct.Matrix.html

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.