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

Adds mineig and maxeig methods #2725

Merged
merged 2 commits into from
Apr 2, 2013
Merged

Adds mineig and maxeig methods #2725

merged 2 commits into from
Apr 2, 2013

Conversation

jiahao
Copy link
Member

@jiahao jiahao commented Apr 1, 2013

Find smallest and largest eigenvalues

Find smallest and largest eigenvalues for real eigenvalues

Includes specialized method fo SymTridiagonal.
@ViralBShah
Copy link
Member

I am not convinced about the utility of these. It would be worth having these, if they did lesser computation than eigvals does, for example. Can't users easily just do max(eigvals(A))?

@ViralBShah
Copy link
Member

Are you going to have special routines in case of banded matrices, where these routines are more efficient that computing all the eigenvalues?

@@ -67,6 +67,11 @@ eig(m::SymTridiagonal) = LAPACK.stegr!('V', copy(m.dv), copy(m.ev))
eigvals(m::SymTridiagonal, il::Int, iu::Int) = LAPACK.stebz!('I', 'E', 0.0, 0.0, il, iu, -1.0, copy(m.dv), copy(m.ev))[1]
eigvals(m::SymTridiagonal, vl::Float64, vu::Float64) = LAPACK.stebz!('V', 'E', vl, vu, 0, 0, -1.0, copy(m.dv), copy(m.ev))[1]
eigvals(m::SymTridiagonal) = LAPACK.stebz!('A', 'E', 0.0, 0.0, 0, 0, -1.0, copy(m.dv), copy(m.ev))[1]

#Computes largest and smallest eigenvalue
maxeig(m::SymTridiagonal) = eigvals(m, size(m)[1], size(m)[1])[1]
Copy link
Member

Choose a reason for hiding this comment

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

size(m, 1) avoids the creation of an array.

Copy link
Member

Choose a reason for hiding this comment

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

I think size returns a tuple, but we should still do size(m,1) to avoid the tuple creation.

Copy link
Member Author

Choose a reason for hiding this comment

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

so this should be eigvals(m, size(m,1), size(m,1))[1]? I'm assuming the last one can't be helped.

@andreasnoack
Copy link
Member

The method for SymTridigonal is optimized. There is also an eigmax method for Hermitian and maybe the method should be defined only for types for which there exist an optimized version, i.e. for symemtric/hermitian matrices.

@ViralBShah
Copy link
Member

I like restricting it to cases where it does something better than the obvious stuff. However, once you get used to this, you will want it to work everywhere.

@ViralBShah
Copy link
Member

@jiahao How about we restrict this only to the cases where there is an optimized version, unless you think otherwise. Also, do update what @andreasnoackjensen pointed out.

@pao
Copy link
Member

pao commented Apr 1, 2013

I like restricting it to cases where it does something better than the obvious stuff.

That seems destined to cause headaches in otherwise generic code.

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

The computation of the extremal eigenvalues shows up all the time in applications, and this essentially implements what @alanedelman was trying to do in #2267.

I'm in favor of providing the generic method, even if purely for convenience. It gives a graceful "obvious" fallback method for the end user, but have it overloaded for Hermitian and SymTridiagonals. I can at least rename maxeig(Hermitian) to eigmax for consistency, since the latter seems to be much more prevalent notation, unless people feel strongly that it should be the other way round.

@ViralBShah
Copy link
Member

Even in the sym tridiagonsl case, is it not just picking out the max value from all eigenvalues, or is it actually taking less computational effort when using eigmax instead of eigvals?

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

@ViralBShah in both Hermitian and SymTridiagonal the eigenvalue searches are restricted to the subspaces specified by the given range of eigenvalue indices, so yes they indeed do less work in the LAPACK routines.

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

In fact for the SymTridiagonal case you can always achieve O(n^2), in good cases even O(n) work

@ViralBShah
Copy link
Member

Thanks for the explanation. Can you rename to eigmax and eigmin? That makes them discoverable with tab completion. Also, let's keep the generic cases too.

@ViralBShah
Copy link
Member

It would be great to get some tests in as well.

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

Oops, I meant maxeig is more prevalent than eigmax (27k Google hits vs 7.4k). I'll look into this later.

@johnmyleswhite
Copy link
Member

I'm not sure if this needs to be in Base, but introducing a method that is usually an alias for max(eigvals(...)), but can be usefully over-riden for special types seems like the poster child for multiple dispatch. Only implementing this when it's faster than a two-part function call seems to set the wrong precedent.

@lindahua
Copy link
Contributor

lindahua commented Apr 1, 2013

Please ignore my previous comment (I did not read the code carefully enough). The current implementation seems to be the optimal one.

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

@johnmyleswhite I am not familiar with Julia enough to fully understand what you are proposing, but feel free to implement it if you think it is a good idea ;)

@johnmyleswhite
Copy link
Member

You've already implemented it. :) I was just advocating your work since it seems like a good example of redefining a function on a special type like SymTridiagonal.

@jiahao
Copy link
Member Author

jiahao commented Apr 1, 2013

Ah, I see. I wasn't sure if you were alluding to some more esoteric feature of Julia that I didn't know about. On a related note I was wondering if type promotion was a more elegant approach to implementing +,-,x, etc. on the specialized matrix types, especially when doing things like SymTridiagonal+Tridiagonal(=Tridiagonal) or Bidiagonal*SymTridiagonal=Matrix

ViralBShah added a commit that referenced this pull request Apr 2, 2013
Adds mineig and maxeig methods
@ViralBShah ViralBShah merged commit 472bc08 into JuliaLang:master Apr 2, 2013
@jiahao jiahao deleted the maxeig branch April 2, 2013 14:50
@jiahao jiahao mentioned this pull request Apr 11, 2013
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.

6 participants