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

Improve `rand` for tangent vectors on `SpecialOrthogonal(2)` #688

Closed olivierverdier closed 9 months ago

olivierverdier commented 9 months ago

If M is defined as [0 1; -1 0], then

SO2 = SpecialOrthogonal(2)
rand(SO2; vector_at=identity_element(SO2)) # always return ±M/sqrt(2)

This contaminates products or semidirect products with SO(2), for instance, tangent vector of SpecialEuclidean(2) are not random either.

olivierverdier commented 9 months ago

Okay, it's random, so the title is wrong, but it is not normally distributed.

mateuszbaran commented 9 months ago

Yes, we should change the distribution to something like M .* randn()

kellertuer commented 9 months ago

I think this is really and nice random – rand ( https://docs.julialang.org/en/v1/stdlib/Random/#Base.rand ) does not say, that the randomness might be samples from a finite set.

But you are right, something more normally distributed would be nicer here. but getting distributions (and hence a proper generic rand) to work, cf. https://github.com/JuliaManifolds/Manifolds.jl/issues/57 is something we have in mind for a really long time, but did not yet have the time to get to.

Here, Mateusz remark should fix this for now at least.

olivierverdier commented 9 months ago

I think this is really and nice random – rand ( https://docs.julialang.org/en/v1/stdlib/Random/#Base.rand ) does not say, that the randomness might be samples from a finite set.

I see, that explains it. I should have been warned by the documentation: Usually [...] a Gaussian-like distribution for non-compact manifolds and tangent vectors, although it is not guaranteed.

So random vectors on a circle group returning zero (#686) is also fine, since it samples from a finite set of one element.

I guess the conclusion is that rand on manifolds is not really dependable for now. I'll try to roll out my own random functions instead. You can disregard these issues about random vector generation, then.

mateuszbaran commented 9 months ago

Well, it should be dependable and return something more reasonable. I will patch both this and CircleGroup.

kellertuer commented 9 months ago

If you have nice random functions, PRs are very welcome. But there is also a reason that we had the idea to start ManifoldMeasures.jl a few years back – these days maybe again more like ManifoldsDistributions.jl – since that business is nontrivial. And then we (just) need someone to approach such a package.

We can not guarantee much by now, but as Mateusz write, we can surely improve them a bit. I personally need the rand functions very often just to get some point (not far beyond memory allocation).

mateuszbaran commented 9 months ago

This should fix both issues: https://github.com/JuliaManifolds/Manifolds.jl/pull/689