Added rust implementation of gaussian elimination #183
Conversation
but back substitution is completely broken
| Matrix { rows, cols, data } | ||
| } | ||
|
|
||
| fn get(&self, row: usize, col: usize) -> f64 { |
There was a problem hiding this comment.
I would use the Index traits for this, it would be a lot more elegant
|
|
||
| impl Matrix { | ||
| fn new(rows: usize, cols: usize) -> Matrix { | ||
| let mut data = Vec::with_capacity(rows*cols); |
There was a problem hiding this comment.
This is just: let data = vec![0.0; rows * cols];
| for k in 0..min(a.cols, a.rows) { | ||
| // Step 1: find the maximum element for this kumn | ||
| let mut max_row = 0; | ||
| let mut max_value = 0.0; |
There was a problem hiding this comment.
This is one of the cardinal sins of maximum finding: you should always start out with the maximum being the 0th element of the list, not an arbitrary low value.
| } | ||
|
|
||
| fn back_substitution(a: &Matrix) -> Vec<f64> { | ||
| let mut soln = Vec::with_capacity(a.rows); |
There was a problem hiding this comment.
Similarly to another comment, this is just vec![0.0; a.rows]
Gathros
added
the
Implementation
|
|
||
| impl Index<(usize, usize)> for Matrix { | ||
| type Output = f64; | ||
| fn index<'a>(&'a self, index: (usize, usize)) -> &'a f64 { |
There was a problem hiding this comment.
You shouldn't need the explicit lifetime param in either Index trait implementation
| impl Index<(usize, usize)> for Matrix { | ||
| type Output = f64; | ||
| fn index<'a>(&'a self, index: (usize, usize)) -> &'a f64 { | ||
| let (row, col) = index; |
There was a problem hiding this comment.
You can just destructure these in the argument list: (row, col): (usize, usize)
|
|
||
| fn gaussian_elimination(a: &mut Matrix) { | ||
| for k in 0..min(a.cols, a.rows) { | ||
| // Step 1: find the maximum element for this kumn |
There was a problem hiding this comment.
oops, got a bit to greedy with a find & replace
| // Loop over all remaining rows | ||
| for i in k+1..a.rows { | ||
| // Step 3: find the fraction | ||
| let fraction = a[(i, k)]/a[(k, k)]; |
There was a problem hiding this comment.
I think rust usually has spaces around operators. Could you run the code through rustfmt so we're sure it follows the standard style?
| impl Index<(usize, usize)> for Matrix { | ||
| type Output = f64; | ||
| fn index(&self, index: (usize, usize)) -> &f64 { | ||
| let (row, col) = index; |
There was a problem hiding this comment.
You left the separate destructuring here
| } | ||
|
|
||
| impl IndexMut<(usize, usize)> for Matrix { | ||
| fn index_mut<'a>(&'a mut self, (row, col): (usize, usize)) -> &'a mut f64 { |
There was a problem hiding this comment.
And the explicit lifetime here :)
| fn main() { | ||
| // The example matrix from the text | ||
| let mut a = Matrix::new(3, 4); | ||
| a.data = vec![2.0, 3.0, 4.0, 6.0, 1.0, 2.0, 3.0, 4.0, 3.0, -4.0, 0.0, 10.0]; |
There was a problem hiding this comment.
I would prefer this to be bassed as the 3rd argument to new, probably as &[f64]
I think representing the matrix as a struct carrying the dimensions is probably the cleanest way to do this. Swap rows is included as a method of the matrix mostly because it deals with pointer math that's sort of out of the scope of the actual algorithm, but there's a pretty good argument for not breaking it out into a method as well.