{{ message }}
Created Sherman Morrison method - #1162
Merged
Merged
Conversation
cclauss
approved these changes
Aug 31, 2019
cclauss
left a comment
cclauss
left a comment
Member
There was a problem hiding this comment.
This is really slick! I like the type hints and assertions to guard against bad data. Nice contribution.
A couple of optional things that I would be interested to see added are doctests and list comprehensions.
1. Added docstring tests 2. Tweaked __str__() using join 3. Added __repr__() 4. Changed index validation to be independent method
Contributor
Author
cclauss
reviewed
Aug 31, 2019
| s += "[" | ||
| s += ", ".join(string_format_identifier % (obj,) for obj in row_vector) | ||
| s += "]\n" | ||
| s += "\n".join("[" + ", ".join(string_format_identifier % (obj,) for obj in row_vector) + "]" for row_vector in self.array) |
Member
There was a problem hiding this comment.
This line can not be read on GitHub without left-right scrolling. Please wrap it so this is no longer the case.
cclauss
reviewed
Aug 31, 2019
| # Validation | ||
| assert(self.row == another.row and self.column == another.column) | ||
| assert(isinstance(another, Matrix)) | ||
| assert(self.row == another.row and self.column == another.column) |
Member
There was a problem hiding this comment.
assert(0)
assert(False)
Lose the parens ()... it is not a function.
1. Reduced line length for __str__() 2. Removed parens for assert
Contributor
Author
|
If you need more things to fix then please write comments. Thank you. |
Contributor
Author
stokhos
pushed a commit
to stokhos/Python
that referenced
this pull request
Jan 3, 2021
* Created Sherman Morrison * Added docstring for class * Updated Sherman morrison 1. Added docstring tests 2. Tweaked __str__() using join 3. Added __repr__() 4. Changed index validation to be independent method * Applied cclauss's point 1. Reduced line length for __str__() 2. Removed parens for assert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I created method to apply Sherman-Morrison formula to Matrix.
Sherman-Morrison formula is important to reduce time complexity on calculating inverse matrix on some special cases. This formula is also used to improve BFGS update performance. For deeper stuffs please look into this documents:
https://en.wikipedia.org/wiki/Sherman%E2%80%93Morrison_formula
https://en.wikipedia.org/wiki/Broyden%E2%80%93Fletcher%E2%80%93Goldfarb%E2%80%93Shanno_algorithm
https://en.wikipedia.org/wiki/Quasi-Newton_method