JuliaManifolds / Manifolds.jl

Manifolds.jl provides a library of manifolds aiming for an easy-to-use and fast implementation.
https://juliamanifolds.github.io/Manifolds.jl
MIT License
367 stars 52 forks source link

Symplectic Grassmann #700

Closed kellertuer closed 7 months ago

kellertuer commented 7 months ago

this PR is a start to implement the (real) Symplectic Grassmann, which one obtains from Symplectic Stiefel the same way one obtains Grassmann from Stiefel. It will probably also revise a few symplectic Stiefel functions.

Both symplectic Stiefel and Grassmann stem from this paper https://arxiv.org/abs/2108.12447

codecov[bot] commented 7 months ago

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (64ae41a) 99.54% compared to head (9500bd5) 94.87%.

Files Patch % Lines
src/manifolds/SymplecticGrassmannStiefel.jl 90.47% 4 Missing :warning:
src/manifolds/Hamiltonian.jl 95.31% 3 Missing :warning:
src/manifolds/Symplectic.jl 97.90% 3 Missing :warning:
src/manifolds/SymplecticGrassmann.jl 92.85% 1 Missing :warning:
src/manifolds/SymplecticGrassmannProjector.jl 96.55% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #700 +/- ## ========================================== - Coverage 99.54% 94.87% -4.67% ========================================== Files 108 112 +4 Lines 10715 10871 +156 ========================================== - Hits 10666 10314 -352 - Misses 49 557 +508 ```

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

kellertuer commented 7 months ago

With the last few methods now I would consider this feature-complete (just not yet test&docs complete), so I moved it from draft to ready (but did not yet set the label).

kellertuer commented 7 months ago

Uff, next time I have to start tests along the way as well, spent much too much time on these today. Now just the projector representation is missing, but since we have no functions for these besides the checks, the only thing to come of with is a point ant a tangent vector.

kellertuer commented 7 months ago

Hm, I just wanted to be nice and define Matrix Multiplications for Hamiltonians, but that introduces 100+ ambiguities :( (I will also lift the boundary for the ambiguity check to include 1.10). What shall we do there best? Not define it for Hamiltonian * abstract Matrix? I will check what Symmetric does.

edit: Went for the easy way: Only defining +-* for Hamiltonian with itself.

mateuszbaran commented 7 months ago

Yes, I think you should only define +-* for Hamiltonian with itself. Alternatively, you can define getindex and rely on the generic definitions.

kellertuer commented 7 months ago

Ah yes in between I got a few getindex errors, but they looked super involved, that it was unclear to me, which variants need to be implemented. For now this should be enough for the start at least.

kellertuer commented 7 months ago

In principle, this is just missing my understanding how to get from a StiefelTVector on SpGr to a ProjectorTVector (to be precise to the specific representation we use).

kellertuer commented 7 months ago

So this one really misses the one test that verifies a correct ProjectorTVector on SpGr, because mainly I still have to come up with a correct one.

kellertuer commented 7 months ago

This now covers all I wanted to be included. I already bumped the version, but I would also like to include the Multinomial PR (#702) in this version I think, since that one is basically finished as well.

mateuszbaran commented 7 months ago

It looks like tolerance in one test for SymplecticStiefel is too low.

kellertuer commented 7 months ago

I am already running this locally. I am a bit surprised by that since I did not change that.