-
Notifications
You must be signed in to change notification settings - Fork 15
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
Beyn rankdrop checking #164
Conversation
Hmm, still seems to not quite capture the right behavior. For example, the following problem: (A+1k)v= λv, where A = diag(1,2,3,4,...) should have eigenvalues (-1,-2,-3,-4,...). But the following triggers a warning: using NonlinearEigenproblems, LinearAlgebra, SparseArrays
N=1000
As = [sparse(1.0I,N,N), spdiagm(0=>1.0*(1:N))]
fns = [identity, one]
nep = SPMF_NEP(As,fns)
contour_beyn(nep;neigs=3, σ=-3, radius=.5) It gives the correct answer (-3 eval) but it should not trigger the warning. I think there needs to be both a measure of a drop in rank (as measured by S[:]/S[1]) as well as an absolute rank tolerance (S[1] > abs_rank_tol to keep). I like the |
I've added an absolute rank tolerance on my branch, and I see now how this logic works, so I slightly revise my earlier comment. I think as written (having added an absolute tolerance), if So, once an absolution rank tolerance is implemented (which handles the case of no enclosed eigenvalues), I think the only scenario that really needs addressing is the case I guess we should think about how this should operate. Should it always throw a warning if |
Ah, I can think of an easy solution. If we treat it like How does that sound? |
Thanks. Indeed there is still some improvements possible. I have changed the logic (and added sorting + only taking max neigs eigpairs) in the commit 182ecb5 With the nep in your example:
It should be consistent with what you write. Did I forget something? Point 1 corresponds to your first example where you wanted no warning. I think a warning that insufficient eigvals were computed is okay. The warning can be suppressed with The sorting would have to be replaced by some form of inside contour checking once we add support for domains other than disks. |
I think this is great, thanks! |
👍 |
This aims to fix #162.
Currently, the rankdrop is only used to decide on what the user recommendation should be in case insufficient eigvals are found.