-
-
Notifications
You must be signed in to change notification settings - Fork 46.9k
Linear algebra/power iteration #2190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linear algebra/power iteration #2190
Conversation
Hey @zakademic, TravisCI finished with status TravisBuddy Request Identifier: a88eb620-c19e-11ea-8e31-7fd6e6dac888 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go through the CONTRIBUTING.md and make sure it passes the flake8 style standard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add doctests
@QuantumNovice Your review please. |
Oops I missed that when writing power iteration. The rayleigh quotient I wrote within it is a duplicate; I will remove. But power iteration is not a duplicate. Thanks! |
OK will do. |
def rayleigh_quotient(input_matrix: np.array, vector: np.array) -> float: | ||
|
||
""" | ||
Rayleigh Quotient. | ||
Fine the Rayliegh quotient of matrix and vector. Given an eigenvector, | ||
the Rayleigh quotient is the corresponding eigenvalue. | ||
|
||
Input | ||
input_matrix: input matrix. | ||
Numpy array. np.shape(input_matrix) == (N,N). | ||
vector: input vector. | ||
Numpy array. np.shape(vector) == (N,) or (N,1) | ||
""" | ||
|
||
num = np.dot(vector.T, np.dot(input_matrix, vector)) | ||
den = np.dot(vector.T,vector) | ||
|
||
return num/den |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove these lines and use this instead?
from rayleigh_quotient import rayleigh_quotient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on this now, thank you!
@QuantumNovice I actually removed Rayleigh quotient since in the power iteration algorithm the vectors are already normalized. This removes a dot product and division operation. I had not written doctests before so I referenced what was happening in Rayleigh Quotient and tried to emulate that. I am using VS Code now and set Thanks all for your help! |
Changed convergence check line. Co-authored-by: Christian Clauss <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Numpy array. np.shape(largest_eigenvector) == (N,) or (N,1). | ||
|
||
>>> import numpy as np | ||
>>> A = np.array([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the same variable names as the function parameters so the reader does not get lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cclauss this is a great point; I made the change :)
eigs, eigvs = np.linalg.eigh(A) | ||
eig_max = eigs[-1] | ||
eigv_max = eigvs[:, -1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Longer variable names would improve readability for learners.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made changes to be more clear and added some comments as well. Appreciate it!
Named tests more clearly. Co-authored-by: Christian Clauss <[email protected]>
Hey @zakademic, TravisCI finished with status TravisBuddy Request Identifier: 1f69e7e0-cf02-11ea-9502-ed86ad10d0d8 |
…octests to match function call.
…akademic/Python into linear_algebra/power_iteration
Nice!! |
* Initial commit of power iteration. * Added more documentation for power iteration and rayleigh quotient * Type hinting for rayleigh quotient * Changes after running black and flake8. * Added doctests, added unit tests. Removed Rayleigh quotient as it is not needed. * Update linear_algebra/src/power_iteration.py Changed convergence check line. Co-authored-by: Christian Clauss <[email protected]> * Update linear_algebra/src/power_iteration.py Named tests more clearly. Co-authored-by: Christian Clauss <[email protected]> * Changed naming in test function to be more clear. Changed naming in doctests to match function call. * Self running tests Co-authored-by: Zeyad Zaky <[email protected]> Co-authored-by: Christian Clauss <[email protected]>
Describe your change:
Added power iteration algorithm. Given a symmetric matrix and a random vector of same space, will find largest eigenvalue and corresponding eigenvector. Rayleigh quotient needed to compute eigenvalue given an eigenvector.
Checklist:
Fixes: #{$ISSUE_NO}
.