Reorganize distributions into submodule

[sethaxen]

As proposed in #57, this PR reorganizes the distributions code to be in its own submodule ManifoldDistributions. The main reason for this is so that we can (eventually) remove Distribution from all manifold distribution names, in favor of simpler names more consistent with Distributions.jl like Projected and Uniform.

The intended usage in the future would be for someone who wants manifold distributions to call using Manifolds.ManifoldDistributions.

There are some open questions here. Right now, only the basic distributions are defined in ManifoldDistributions, while manifold-specific distributions like PowerPointDistribution are defined in that manifold file. If we instead move those manifold-specific distributions into ManifoldDistributions, we might speed up the load time of Manifolds quite a bit, and it would make it easier to split ManifoldDistributions into its own package eventually if that's what we want to do.

Edit: I'd appreciate input on this latter point

[codecov]

Codecov Report

Merging #143 into master will increase coverage by 1.54%.
The diff coverage is 82.02%.

@@            Coverage Diff             @@
##           master     #143      +/-   ##
==========================================
+ Coverage   96.14%   97.68%   +1.54%     
==========================================
  Files          48       49       +1     
  Lines        3213     3155      -58     
==========================================
- Hits         3089     3082       -7     
+ Misses        124       73      -51     

Impacted Files	Coverage Δ
src/Manifolds.jl	100.00% <ø> (ø)
src/manifolds/Euclidean.jl	100.00% <ø> (+2.29%)	⬆️
src/manifolds/MetricManifold.jl	100.00% <ø> (+11.90%)	⬆️
src/manifolds/PowerManifold.jl	96.91% <ø> (+0.67%)	⬆️
src/manifolds/ProductManifold.jl	97.48% <ø> (+7.78%)	⬆️
src/manifolds/Rotations.jl	95.28% <ø> (+0.08%)	⬆️
src/manifolds/Sphere.jl	100.00% <ø> (ø)
src/distributions/manifold_distribution.jl	80.00% <80.00%> (ø)
src/distributions/projected_distribution.jl	92.30% <92.30%> (ø)
src/distributions/ManifoldDistributions.jl	66.66% <100.00%> (ø)
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fa3d4f...214bc4a. Read the comment docs.

[kellertuer]

I think it's a good idea to do a separate ManifoldDistributions package in general. What I am not so sure about is, whether that can be done using “just” ManifoldsBase in order to keep dependencies low, or whether it has to use both ManifoldsBase and Manifolds (Manopt currently needs both, but I am working on that).

This might be a counter argument to the idea of defining manifold-specific distributions in distributions, so we have to see what's more practical.

For the PowerDistribution however, I still think that ProductManifold and ProductManifold would be nice to have in ManifoldsBase anyways.

Starting with a submodule is a good idea!

Edit: With the label “WIP” instead of in the title, I feel its easier to filer and see (and more colourful orange).

[mateuszbaran]

Separating distributions is a good idea and I generally support even splitting it to a separate package. ManifoldDistribution would, however, have to depend on Manifolds.jl (some distributions could easily be implemented using just ManifoldsBase but distributions like the ones from #142 couldn't. So, if we really want to split distributions, all manifold-specific distributions would have to be moved to the ManifoldDistributions module.

[sethaxen]

ManifoldDistribution would, however, have to depend on Manifolds.jl (some distributions could easily be implemented using just ManifoldsBase but distributions like the ones from #142 couldn't. So, if we really want to split distributions, all manifold-specific distributions would have to be moved to the ManifoldDistributions module.

The alternative to this is to keep everything in ManifoldDistributions generic and use Requires for manifold-specific distributions and overloads in Manifolds. But for that to work, all of that distribution-specific code would need to be separate from the manifolds themselves, so there's not much benefit to having it in Manifolds.

For now, I think I'll move all distributions code, including manifold-specific distributions, into the ManifoldDistributions submodule. Then I'll simplify the type names, and that should be it for this PR.

[kellertuer]

Good plan. We can still decide later whether to turn that in to its own module (I think that might be nice) if we find a good way to split.

An idea could also be ManifoldsDistributions provides all generic code (and builds on ManifoldsBase), the specific code is in Manifolds (depends on base & distributions), but that specific code is short and simple (hopefully, with all generic code prepared already).

But we can disuss and decide this later and do this PR as a submodule, for sure.

[sethaxen]

Anyone have any good ideas for a nice file structure in ManifoldDistributions? What I have now is really unsatisfactory, but I can't think if anything better.

[mateuszbaran]

I think the file structure is OK. At least I don't have a better idea.

[sethaxen]

This is obviously too stale to continue work on it. I'll leave this open now as a reminder to try again when I have time.

Unless someone else wants to take a stab at it.

[mateuszbaran]

Sure, no need to rush it. I won't work on this for now. I want to finish porting FunManifolds and find a good solution to JuliaManifolds/ManifoldsBase.jl#45 first.

[kellertuer]

Sure, you can also close this one and keep track of this in an issue, since I think in som time this might be hard to get back up to date? But whatever you find nicer.

[kellertuer]

This PR might be obsolete since we started ManifoldMeasures.jl?

[sethaxen]

This PR might be obsolete since we started ManifoldMeasures.jl?

Yes, this PR is way too outdated to be useful, and I have no intention of continuing this work in another PR.