Skip to content
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

Introduce Weingarten and riemannian_Hessian for a few manifolds #644

Merged
merged 57 commits into from
Aug 30, 2023

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Aug 17, 2023

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 JuliaManifolds/ManifoldDiff.jl#30 to be merged

  • Sphere (Weingarten! implemented, riemannian_Hessian! not necessary, since with Weingarten we have the efficient variant already)
  • Symmetric positive Definite (riemannian_Hessian!) from Nguyen (Eq. 7.3)
  • Stiefel (Weingarten! implemented, riemannian_Hessian! Eq. (5.6) Nguyen)
  • Stiefel Canonical and Alpha
  • SpecialOrthogonal (Weingarten! implemented, riemannian_Hessian! the same as Stiefel, just that tangent vectors are in the Lie algebra)
  • Grassmann (riemannian_Hessian!, Nguyen, Eq. 6.6)
  • FixedRankMatrices (riemannian_Hessian!, Vandereycken 2013, though I did not adapt his longish formula but merely adopted his code)
  • Hyperbolic space (riemannian_Hessian!, adapting Remark 4.1 Nguyen hopefully correctly)
  • ...check other ehess2rhess functions in Manopt
  • Documentation (especially Literature rendering, most formula should be present by now)
  • Further to check / verify
    • Does our current implementation work with complex (Stiefel/Grassmann/Sphere)?
    • Power and product manifold
    • Oblique (should work with power, but add test)
    • positive numbers
    • unitary (adapt rotation)
  • Test coverage.
  • Moved References to references.bib.

…garten! for the Sphere (no special riemannian_Hessian required).
@kellertuer kellertuer added documentation Improvements or additions to documentation enhancement New feature or request extend manifold This issue proposes/asks for new functions to extend an existing manifold preview docs Add this label if you want to see a PR-preview of the documentation labels Aug 17, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

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

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

@kellertuer
Copy link
Member Author

kellertuer commented Aug 18, 2023

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
Copy link
Member

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
Copy link
Member Author

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

src/manifolds/HyperbolicHyperboloid.jl Outdated Show resolved Hide resolved
src/manifolds/HyperbolicHyperboloid.jl Outdated Show resolved Hide resolved
@kellertuer
Copy link
Member Author

kellertuer commented Aug 20, 2023

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
Copy link
Member Author

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

Copy link
Member

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small comments.

docs/src/misc/internals.md Outdated Show resolved Hide resolved
src/manifolds/FixedRankMatrices.jl Outdated Show resolved Hide resolved
src/manifolds/FixedRankMatrices.jl Outdated Show resolved Hide resolved
src/manifolds/HyperbolicHyperboloid.jl Outdated Show resolved Hide resolved
src/manifolds/Rotations.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
src/utils.jl Outdated Show resolved Hide resolved
test/manifolds/projective_space.jl Outdated Show resolved Hide resolved
test/manifolds/rotations.jl Outdated Show resolved Hide resolved
kellertuer and others added 4 commits August 30, 2023 09:05
Co-authored-by: Mateusz Baran <mateuszbaran89@gmail.com>
….com:JuliaManifolds/Manifolds.jl into kellertuer/weingarten-and-Hessian-conversion
@mateuszbaran
Copy link
Member

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
Copy link
Member Author

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.

Copy link
Member

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

kellertuer and others added 9 commits August 30, 2023 14:24
# Conflicts:
#	docs/src/manifolds/probabilitysimplex.md
#	docs/src/misc/notation.md
#	src/Manifolds.jl
#	src/manifolds/Euclidean.jl
#	src/manifolds/HyperbolicHyperboloid.jl
#	src/manifolds/PowerManifold.jl
#	src/manifolds/ProductManifold.jl
#	src/manifolds/Sphere.jl
#	src/manifolds/SymmetricPositiveDefiniteAffineInvariant.jl
#	test/manifolds/euclidean.jl
#	test/manifolds/hyperbolic.jl
#	test/manifolds/positive_numbers.jl
#	test/manifolds/product_manifold.jl
#	test/manifolds/rotations.jl
#	test/manifolds/sphere.jl
#	test/manifolds/symmetric_positive_definite.jl
….com:JuliaManifolds/Manifolds.jl into kellertuer/weingarten-and-Hessian-conversion
@kellertuer kellertuer merged commit 805d050 into master Aug 30, 2023
26 of 30 checks passed
@kellertuer kellertuer deleted the kellertuer/weingarten-and-Hessian-conversion branch May 4, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request extend manifold This issue proposes/asks for new functions to extend an existing manifold preview docs Add this label if you want to see a PR-preview of the documentation Ready-for-Review A label for pull requests that are feature-ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants