-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add balancing options for eigenvector calculations #5428
Conversation
I just read this post last month about how automatic balancing can be bad. |
|
||
Compute the eigenvalue decomposition of ``A`` and return an ``Eigen`` object. If ``F`` is the factorization object, the eigenvalues can be accessed with ``F[:values]`` and the eigenvectors with ``F[:vectors]``. The following functions are available for ``Eigen`` objects: ``inv``, ``det``. | ||
Compute the eigenvalue decomposition of ``A`` and return an ``Eigen`` object. If ``F`` is the factorization object, the eigenvalues can be accessed with ``F[:values]`` and the eigenvectors with ``F[:vectors]``. The following functions are available for ``Eigen`` objects: ``inv``, ``det``. For general non-symmetric matrices it is possible to specify how the matrix is balanced before the eigenvector calculation. Possible values are ``:balance``(default), ``:permute``,``:diagonal`` and ``nobalance``. |
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.
Missing colon: :nobalance
?
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.
Thank you.
@jiahao That was also my inspiration for adding the option. |
I have updated the pull request so I think it is ready for merge. |
Add balancing options for eigenvector calculations
- Include issue number in NEWS.md - Added missing colons in error message - More detailed explanation of balance options and what they do in the help text.
Using a symbol seems really weird for this interface. Why not just make it a boolean - |
There are four balancing options. |
I do see that now. I keep wishing we had a nicer way to do this, that is as compact, but maybe this is just fine. |
Actually the options are the product of two binary options, diagonal scaling and permuting, so we could have |
I like the idea of two orthogonal boolean options rather than one four-way option. Not super thrilled about the name |
I have wrapped the expert drivers for general eigenvalue calculations and made balance options in the eig functions.