Implement NumRuby::Linalg #30
Conversation
| .rake_tasks~ | ||
| *.so | ||
| *.so | ||
| .vscode |
There was a problem hiding this comment.
I think you shouldn't add this here, you could place this in a .global-gitignore as you can see here https://gist.github.com/subfuzion/db7f57fff2fb6998a16c WDYT? 🤔
There was a problem hiding this comment.
I think *.so should be in .gitignore, .vscode can be shifted to .global-gitignore.
| def self.inv(obj) | ||
| if obj.is_a?(NMatrix) | ||
| return obj.invert | ||
| def self.inv(matrix) |
There was a problem hiding this comment.
I think you could put this in functions. For example:
def self.inv(matrix) is_a_matrix?(matrix) valid_shape?(matrix, 2) ... end private def is_a_matrix?(matrix) raise("Invalid matrix. Not of type NMatrix.") unless matrix.is_a?(NMatrix) end def valid_shape?(matrix, shape) raise("Invalid shape of matrix. Should be 2.") unless matrix.dim != shape endWDYT? 🤔
There was a problem hiding this comment.
Yeah, seems alright. Functions for these would be a good idea as these are quite frequent. I'll add these once the PR is complete. Thanks.
| if matrix.dim != 2 | ||
| raise("Invalid shape of matrix. Should be 2.") | ||
| end | ||
| if matrix.shape[0] != matrix.shape[1] |
There was a problem hiding this comment.
so I think you could put this in a function as:
def square_matrix?(matrix) raise("Invalid shape. Expected square matrix.") unless matrix.shape[0] != matrix.shape[1] endWDYT? 🤔
| if matrix.shape[0] != matrix.shape[1] | ||
| raise("Invalid shape. Expected square matrix.") | ||
| end | ||
| m, n = matrix.shape |
There was a problem hiding this comment.
m is rows count and n is columns count.
There was a problem hiding this comment.
I think I could name these fields as row and account_ columns to make this more readable. WDYT? 🤔
| lu, ipiv = NumRuby::Lapack.getrf(matrix) | ||
| inv_a = NumRuby::Lapack.getri(lu, ipiv) | ||
| | ||
| return inv_a |
There was a problem hiding this comment.
As this is the last line in this function, it isn't necessary to put return. You could put inv_a and this variable is automatically returned.
There was a problem hiding this comment.
I believe including return makes it more readable.
| m, n = matrix.shape | ||
| | ||
| if pivoting == false | ||
| if pivoting == true |
There was a problem hiding this comment.
I think you could put this as:
if pivoting ... endWDYT? 🤔
| r = NumRuby.triu(matrix[0...n, 0...n]) | ||
| end | ||
| | ||
| if pivoting == true |
There was a problem hiding this comment.
This validation is similar to line 168. I think you could put this in a function. WDYT? 🤔
There was a problem hiding this comment.
A function for an if statement?
There was a problem hiding this comment.
🤔 yes, maybe move only the conditional is weird, but I think that you could the conditional blocks in functions. WDYT? 🤔
3 participants
No description provided.