-
Notifications
You must be signed in to change notification settings - Fork 39
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
Remove unused code (reference to undefined msg
)
#117
Conversation
This should in principle be caught by tests...
msg
msg
)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #117 +/- ##
==========================================
- Coverage 88.07% 87.97% -0.10%
==========================================
Files 32 32
Lines 3395 3393 -2
==========================================
- Hits 2990 2985 -5
- Misses 405 408 +3 ☔ View full report in Codecov by Sentry. |
I guess I was lazy and did not actually test the output: From test/eigsolve.jl line 27-31: alg = Lanczos(; orth=orth, krylovdim=n, maxiter=1, tol=tolerance(T),
verbosity=4)
@test_logs min_level = Logging.Warn eigsolve(wrapop(A, Val(mode)),
wrapvec(v, Val(mode)),
n1, :SR, alg) This doesn't even capture the error because the whole |
src/eigsolve/arnoldi.jl
Outdated
if eltype(fact) <: Real && krylovdim < 4 | ||
error("krylov dimension should be at least 4 to avoid getting stuck in the Arnoldi process in real arithmetic") | ||
elseif krylovdim < 2 | ||
error("krylov dimension should be at least 2 to avoid getting stuck in the Arnoldi process in complex arithmetic") | ||
end | ||
|
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.
Does this mean that we cannot factorize small matrices using KrylovKit? It would have some additional complications for working with TensorKit, where the blocks could end up quite small.
Ok, I think this is ready @lkdvos . Will you merge? |
I don't have the rights to approve this, but good to go for me! |
This should in principle be caught by tests...