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
368 stars 53 forks source link

Introduce `Weingarten` and `riemannian_Hessian` for a few manifolds #644

Closed kellertuer closed 1 year ago

kellertuer commented 1 year ago

This will allow for the same functionality that ehess2rhess does in Manopt.

This also introduces DocumenterCitations at least for any new thing we cite. It also requires https://github.com/JuliaManifolds/ManifoldDiff.jl/pull/30 to be merged

codecov[bot] commented 1 year ago

Codecov Report

Merging #644 (431c1ab) into master (7afc7af) will increase coverage by 0.01%. Report is 1 commits behind head on master. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #644      +/-   ##
==========================================
+ Coverage   99.20%   99.22%   +0.01%     
==========================================
  Files         106      106              
  Lines       10215    10430     +215     
==========================================
+ Hits        10134    10349     +215     
  Misses         81       81              
Files Changed Coverage Δ
src/Manifolds.jl 86.66% <ø> (ø)
src/groups/connections.jl 100.00% <ø> (ø)
src/groups/general_linear.jl 99.09% <ø> (ø)
src/groups/group.jl 100.00% <ø> (ø)
src/groups/heisenberg.jl 100.00% <ø> (ø)
src/groups/rotation_action.jl 100.00% <ø> (ø)
src/groups/special_euclidean.jl 99.60% <ø> (ø)
src/groups/unitary.jl 100.00% <ø> (ø)
src/manifolds/CenteredMatrices.jl 100.00% <ø> (ø)
src/manifolds/CholeskySpace.jl 100.00% <ø> (ø)
... and 47 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kellertuer commented 1 year ago

I fear I am getting to my limits concerning reverse engineering. For example on Stiefel we now have Weingarten!, but ehess2rhess in Matlab reads like

https://github.com/NicolasBoumal/manopt/blob/7c4cb3832b81fd0f001d62d91cd504f533fbcc1d/manopt/manifolds/stiefel/stiefelfactory.m#L117-L123

I do not see why riemannian_Hessian should simplify to this and it is not documented where this (it seems shorter) form is from. So I can write that in our notation still as projecting eH - X*1/2*(p'*eG + eG'*p) and I could implement that and check whether it yields the same as riemannian_Hessian(M, p, eG, eH, X) (which uses another project and Weingarten call). So numerically I could check that this is valid as a maybe shorter, maybe more efficient variant. But I would not be able to document that properly where it is from (besides stating it is from Matlab, but then one does not know where that is from)...

So what do we best do here?

mateuszbaran commented 1 year ago

This paper has a few formulas: https://arxiv.org/pdf/2009.10159v2.pdf (for example Stiefel is (5.6) and you can probably derive Grassmann from (6.6), and SPD is (7.3)).

kellertuer commented 1 year ago

At first glance even better – I also understand what the paper writes :)

kellertuer commented 1 year ago

I think I now understood most from my list, wrote to Nguyen about a clarification whether I understood Hyperbolic correctly. For Fixedrank, I found a formula in Vandereycken, 2013, but since that was very complicated, I did not adapt it to our notation, but merely adopted his code provided in Matlab to our setting.

Documentation (correction typos) and Testing still missing

kellertuer commented 1 year ago

There still exist a few more conversions I can see in code, but not yet their derivation / formulae. So I'll leave them for later.

kellertuer commented 1 year ago

I just took the time to transfer roughly half of our footnotes already into the new BibTeX format I introduced here (similar to Manopt.jl), but I am not sure I will finish this on this PR (it was mainly while waiting for a test).

kellertuer commented 1 year ago

This PR started using DocumenterCitations and I just moved all footnote-references to the bib file,

Currently we should still check for local literatures maybe (i.e. to print one on every pages that contains references) and in the long run the formatting of the literature could be improved but having that more automated is already very nice.

kellertuer commented 1 year ago

Since I now also finished the tutorial on Manopt, I am convinced this is really in the form we could merge this – the Manopt PR will wait for this. And sure, that does not mean we should rush this.

kellertuer commented 1 year ago

Where the dimensions were correct, I used symmetrisation in place of Y otherwise one allocation. It made a few code lines even much nicer :)

mateuszbaran commented 1 year ago

A small possible TODO for later: generalize https://github.com/JuliaManifolds/ManifoldDiff.jl/pull/30/files#diff-9a324d63614dd422d4d2b47ac0937ea275997adba0771b8c467ecc9231cb9f41R365-R369 to Nguyen's (3.11). I don't think we have all the necessary functions though.

kellertuer commented 1 year ago

I am also not so sure it is that easy, since (3.11) is a bit more involved. But sure having that generically would be quite cool.