Jutho / KrylovKit.jl

Krylov methods for linear problems, eigenvalues, singular values and matrix functions
Other
284 stars 37 forks source link

VectorInterface #65

Closed lkdvos closed 7 months ago

lkdvos commented 1 year ago

rewrite algorithms in terms of VectorInterface methods.

codecov[bot] commented 1 year ago

Codecov Report

Attention: Patch coverage is 81.79272% with 65 lines in your changes are missing coverage. Please review.

Project coverage is 82.05%. Comparing base (e4932b4) to head (7a27157).

Files Patch % Lines
src/innerproductvec.jl 0.00% 18 Missing :warning:
src/recursivevec.jl 51.85% 13 Missing :warning:
src/adrules/linsolve.jl 33.33% 10 Missing :warning:
src/eigsolve/golubye.jl 87.03% 7 Missing :warning:
src/factorizations/lanczos.jl 82.35% 6 Missing :warning:
src/factorizations/gkl.jl 87.50% 4 Missing :warning:
src/orthonormal.jl 95.23% 3 Missing :warning:
src/linsolve/bicgstab.jl 94.87% 2 Missing :warning:
src/matrixfun/expintegrator.jl 87.50% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #65 +/- ## ========================================== - Coverage 82.30% 82.05% -0.25% ========================================== Files 27 27 Lines 2741 2753 +12 ========================================== + Hits 2256 2259 +3 - Misses 485 494 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jutho commented 1 year ago

Hi Lukas; thanks for this great work. I was indeed considering switching to bangbang methods when adopting VectorInterface, so if you have time to try this out and see if there would be any unexpected consequences or complications, feel free to do so ...

lkdvos commented 1 year ago

@Jutho I seem to be stuck with the geneigsolve for out-of-place vectors, although I can't seem to locate what's going wrong. It produces matrices that are not positive definite, such that geneigh! fails, but I have absolutely no clue why this would be different for the out-of-place methods. I might look into it later this week, but any help is definitely welcome. (tests are ran by just commenting out modules "svdsolve.jl" and "expintegrator.jl")

lkdvos commented 7 months ago

Some small remaining remarks:

lkdvos commented 7 months ago

So, if the lights turn green, this should be ready for review.

I excluded AD rules from the new functionality, because we should probably rewrite that section anyways, using Package extensions.

Otherwise, I think everything should now work with VectorInterface, and the tests have been reorganised to extensively test everything with regular vectors, and then do some small additional tests with MinimalVec-type vectors.

In principle I think we can also remove/deprecate RecursiveVec and InnerProductVec, but this would probably require some deprecation update first?

Jutho commented 7 months ago

Nightly failures seem due to Zygote failing?

lkdvos commented 7 months ago

I think so, it also seems to vary quite a bit depending on what version of nightly, so I think we can ignore this. We'll refactor that part anyways when we move the AD support to a package extension I guess.