Skip to content
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 a trivial case for eigen values and vectors #3017

Closed
wants to merge 2 commits into from

Conversation

bartekleon
Copy link
Contributor

partially fixes #3014 #2879

@bartekleon
Copy link
Contributor Author

This is not ultimate bug fix nor solution. It still needs much more testing, but this should solve most general case / bug

@bartekleon
Copy link
Contributor Author

bartekleon commented Aug 23, 2023

@been-jamming @nguyenvukhang if you could check if this patch solves your issues / give more examples where it does not work. Thanks

Also if you have explanation/ideas on to how case like [ [2.0, 1.0], [0.0, 2.0] ] should be handled - having only one solution (should it be [1, 0] vector or [0, 1] and how to determine it?)

Also if you maybe know the general solution for knowing if it should be transposed (in case of [[1, 2], [4, 3]])

Other than these two cases I don't think I had strange results

@been-jamming
Copy link

@bartekleon From what I can see, it looks like you have added a special case for computing the eigenvectors and eigenvalues of a 2x2 matrix? I ran into the issue when computing eigenvalues of much larger matrices, so I'm not sure if this is a good solution.

Also if you have explanation/ideas on to how case like [ [2.0, 1.0], [0.0, 2.0] ] should be handled - having only one solution (should it be [1, 0] vector or [0, 1] and how to determine it?)

If there is only one eigenvalue and one eigenvector, then ans.values and ans.vectors should only contain one entry. But it will probably require work to distinguish between matrices with a single eigenvalue and ones with multiple close eigenvalues. I don't do much numerical linear algebra, so I wouldn't know the best way of going about solving these problems.

Also if you maybe know the general solution for knowing if it should be transposed (in case of [[1, 2], [4, 3]])

I'm not sure what you mean by this. I mixed up the matrix and its transpose when I created the issue.

Other than these two cases I don't think I had strange results

I ran into the issue when computing the eigenvectors of a 101x101 matrix, so I think there is something larger wrong with the algorithm used to compute the eigenvectors. In all cases I've tested, the eigenvalues have been correct.

@gwhitney
Copy link
Collaborator

gwhitney commented Sep 2, 2023

I don't think it's worth adding a special case to the code to fix non-bug #3014. I will take a look at #2879.

@gwhitney
Copy link
Collaborator

gwhitney commented Sep 2, 2023

#2879 is a problem at all matrix sizes, so again I don't think adding a special case for 2x2 is the answer. Recommend closing this PR.

@gwhitney
Copy link
Collaborator

gwhitney commented Sep 2, 2023

Moreover, this PR duplicates the identical function eigenvalues2x2 between complexEigs.js and trivialCase.js, which would have to be fixed if it were to move forward.

@josdejong
Copy link
Owner

Agree, I think handling one special case for 2x2 matrices doesn't solve the issue in general. I'll close this PR now. Thanks anyway for working out this PR @bartekleon and thinking along to find a solution!

@josdejong josdejong closed this Sep 6, 2023
Sign up for free to 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.

eigs function b̶r̶o̶k̶e̶n̶ has a confusing interface
4 participants