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
366 stars 52 forks source link

is_flat(::CholeskySpace) and is_flat(::MetricManifold{ℝ,<:SymmetricPositiveDefinite,LogCholeskyMetric}) return false #684

Closed sjacobsson closed 9 months ago

sjacobsson commented 9 months ago

is_flat() returns false for both CholeskySpace and SPD with the LogCholeskyMetric, but they are flat manifolds. They are flat because distance(M::CholeskySpace, p, q) is of the form $ds^2 = f_1(x_1) dx_1^2 + \dots + f_k(x_k) dx_k^2$, and so it is a reparametrization of $\mathbb{R}^k$. See Lin 2019 Proposition 8 https://arxiv.org/pdf/1908.09326.pdf.

olivierverdier commented 9 months ago

Proposition 8 seems to say that the sectional curvature is zero, not the full curvature tensor, and is_flat returns true only if the full tensor is zero.

sjacobsson commented 9 months ago

The sectional curvature being zero everywhere and the full curvature tensor being zero everywhere are equivalent. For example proposition 8.36 in Lee's Riemannian geometry textbook: A Riemannian metric $g$ has constant sectional curvature $c$ iff its curvature tensor satisfies $R = \frac{1}{2} c g \wedge g$. For the same reasons, I think is_flat(::SymmetricPositiveSemidefiniteFixedRank) should return true.

kellertuer commented 9 months ago

Thanks for stepping in Olivier, but I think with Prop 8.36 Lee, we should return true here for the two cases in the title – I am not directly sure for the positivesemidefinite third case mentioned in the last comment above.

@SimonKvantdator can you maybe do a PR? e can surely help with the details (test coverage, formatting, changelog) :)

sjacobsson commented 9 months ago

@kellertuer, I am not very proficient in git/github but I will try to do a pull request!

As mentioned, I am unsure about SymmetricPositiveSemidefiniteFixedRank. But here is a script I sometimes use to numerically test the curvature of my manifolds:

p = rand(M)

u = rand(M, vector_at=p)
u = u / norm(M, p, u)
v = rand(M, vector_at=p)
v = v - inner(M, p, u, v) * u
v = v / norm(M, p, v)

r = 1e-3
ps = [exp(M, p, r * (cos(theta) * u + sin(theta) * v)) for theta in 0.0:1e-5:(2 * pi)]
ds = [distance(M, p1, p2) for (p1, p2) in zip(ps, [ps[2:end]..., ps[1]])]
C = sum(ds)
K = 3 * (2 * pi * r - C) / (pi * r^3)
println(K)

It uses this formula https://en.wikipedia.org/wiki/Bertrand%E2%80%93Diguet%E2%80%93Puiseux_theorem

olivierverdier commented 9 months ago

I've read the paper now. The SPD manifold is diffeomorphic to a vector space so, sure, you can use the flat metric in that space and map it to the SPD manifold. Then, equipped with that metric, yes, the SPD manifold is flat. That's the essence of the paper. Proposition 3, at least the part showing that the curvature vanishes, is essentially trivial.

So, yes, the space of SPD matrices equipped with the LogCholesky metric should definitely be marked as flat.

For the fixed rank SPD manifold, it is most probably not isomorphic to any vector space, nor any flat manifold, so having is_flat(::SymmetricPositiveSemidefiniteFixedRank) is likely never going to be true.

sjacobsson commented 9 months ago

For the fixed rank SPD manifold, it is most probably not isomorphic to any vector space, nor any flat manifold, so having is_flat(::SymmetricPositiveSemidefiniteFixedRank) is likely never going to be true.

I haven't looked at it too much, I was just guessing that it's flat since exp is implemented as $\mathrm{exp}_q ​Y = q + Y$.

kellertuer commented 9 months ago

@kellertuer, I am not very proficient in git/github but I will try to do a pull request!

That would be great – first of all such that you are listed among the collaborators that help with the code base and to learn about air and GitHub. I can also do that myself, but that might take a while, since the next two weeks at least will be a bit busy on my side.

As mentioned, I am unsure about SymmetricPositiveSemidefiniteFixedRank.

I will have to check literature on this one and we should add those pointers to literature then also to the docs, since we can reference literature in there thanks to using DocumenterCitations.jl

I haven't looked at it too much, I was just guessing that it's flat since exp is implemented as $\exp_q​Y=q+Y$.

I would not phrase it as strong as Olivier, but I am not sure we are on a sub manifold of the fixed rank matrices that is flat here, I would have to check literature on this (or kindly ask you to check).

sjacobsson commented 9 months ago

(or kindly ask you to check).

For sure, I was going to read Massart's article either way tomorrow. I'll just keep this in mind.

In the meantime I'll create a pull request for the two manifolds listed in the title!

kellertuer commented 9 months ago

If you have read Estelle Massart's paper, feel free to add that one (with a similar reference as Mateusz did for the others) to your PR as well :)

sjacobsson commented 9 months ago

I have read Massart's paper, and is seems indeed like the manifold is not flat. See Thm A.18. I'll add this reference to the PR.

I have a few things left to fit together for my own understanding tho,

  1. How does $\mathrm{exp}_q Y = q + Y$ make sense for a curved manifold
  2. In (36) in Massart, she gives the curvature tensor in terms of Lie brackets on $\mathbb{R}^{n \times k}$, aren't they 0??
mateuszbaran commented 9 months ago

Do you have a link to Massart's paper? I'd like to take a look too. $\exp_q Y = q + Y$ can make sense for a curved manifold when it is not an embedded submanifold of Euclidean space, but rather a linear subspace with a different inner/meric.

olivierverdier commented 9 months ago
1. How does expqY=q+Y make sense for a curved manifold

It may be because of the representation of both points and vectors in a bigger manifold, of which the manifold of fixed rank matrices is a quotient. That would make a lot of sense. The fixed rank matrix is not $q$ itself, but the product $π(q) := q q^*$.

kellertuer commented 9 months ago

Do you have a link to Massart's paper? I'd like to take a look too. expq⁡Y=q+Y can make sense for a curved manifold when it is not an embedded submanifold of Euclidean space, but rather a linear subspace with a different inner/meric.

We already have it in our references, its this one https://epubs.siam.org/doi/10.1137/18M1231389

olivierverdier commented 9 months ago

From the documentation:

An element is represented by $q ∈ 𝔽^{n × k}$ from the factorization $p = qq^{\mathrm{H}}$.

It's also consistent with what is in the paper.

There is no contradiction: essentially, the manifold is the nonlinear projection of a linear space. In fact, it seems to be a Riemannian submersion, so horizontal geodesics are projected to geodesics on the quotient manifold. That explains the formula. The bigger space of $n\times p$ matrices is flat, the quotient space is not.

sjacobsson commented 9 months ago

yeah that's probably it, thanks!