Skip to content

Created Sherman Morrison method - #1162

Merged
cclauss merged 4 commits into
TheAlgorithms:masterfrom
McDic:master
Sep 3, 2019
Merged

Created Sherman Morrison method#1162
cclauss merged 4 commits into
TheAlgorithms:masterfrom
McDic:master

Conversation

@McDic

Copy link
Copy Markdown
Contributor

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

@McDicMcDic changed the title Created Sherman MorrisonCreated Sherman Morrison methodAug 31, 2019

@cclausscclauss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@McDic

Copy link
Copy Markdown
ContributorAuthor

@cclauss I added doctests.

By the way, I'm sorry, what do you mean for list comprehension? Do you want some method to iterate over all elements in matrix as single 1x1 element or row/column vector?

Comment threadmatrix/sherman_morrison.py Outdated
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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can not be read on GitHub without left-right scrolling. Please wrap it so this is no longer the case.

Comment threadmatrix/sherman_morrison.py Outdated
# 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)

@cclausscclaussAug 31, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(0)

assert(False)

Lose the parens ()... it is not a function.

1. Reduced line length for __str__()
2. Removed parens for assert
@McDic

McDic commented Aug 31, 2019

Copy link
Copy Markdown
ContributorAuthor

If you need more things to fix then please write comments. Thank you.

@McDic

McDic commented Sep 3, 2019

Copy link
Copy Markdown
ContributorAuthor

@cclauss Is there anything needed to change?

@cclauss
cclauss merged commit 9492e7a into TheAlgorithms:masterSep 3, 2019
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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@McDic@cclauss