Multinomial randomness

[kellertuer]

This PR introduces random for MultinomialSymmetric and MultinomialDoublyStochatstic, which I saw was missing due to a comment here.

Checking the corresponding paper https://ieeexplore.ieee.org/document/8861409
I saw they also introduce MultinoialSPD, which I added here as well. So this short side-track still needs

ToDo

• riemannian_gradient for Multinomial and the three doubly stochastic ones
• riemannian_Hessian for MultinomialSymmetric
• tests
• read docs

The paper (its tech report we link in the docs to be precise) does state the Hessian for MultinomialDoublyStochastic, but it is quite involved (the symmetric one already is), that I am not sure I can “hack that down” in an efficient and stable way.

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (95e90c8) 99.57% compared to head (3b3b4e8) 99.57%.

Files	Patch %	Lines
.../manifolds/MultinomialSymmetricPositiveDefinite.jl	97.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #702      +/-   ##
==========================================
- Coverage   99.57%   99.57%   -0.01%     
==========================================
  Files         112      113       +1     
  Lines       10876    10961      +85     
==========================================
+ Hits        10830    10914      +84     
- Misses         46       47       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kellertuer]

All things I wanted to implement (and that needed fixing) are implemented (and fixed, e.g. the multinomalsymmetric ones have more than just the symmetric check for points now – again).

So this is no draft any longer, but still needs the tests to be checked and extended.

[kellertuer]

Multinomial is failing on 1.6, but it seems to not be so easy to reproduce, on my machine that works fine.

[mateuszbaran]

This is weird. is_vector at /ManifoldsBase/src/decorator_trait.jl:481 assumes that the error returned by check_vector has field val but ComponentManifoldError obviously doesn't have it. It looks like a problem with that is_vector perhaps?

[kellertuer]

Might also be that, but even more strange is, that there should not happen any error and on Julia 1.10, 1.9 and 1.6 on my machine it also does not happen.

[mateuszbaran]

Probably tolerance is too low. It would be more clear if that is_vector didn't try to access val or had a contingency in case it's not present.

[kellertuer]

Yes sure, that would help, but I do not yet directly see, why that is happening yet (also because I can not yet reproduce it here).

[kellertuer]

The problem seems to be that this line

https://github.com/JuliaManifolds/ManifoldsBase.jl/blob/65780dc51475b255258dfd35a225f09a0cd2810b/src/ManifoldsBase.jl#L769

either there or in the decorator at

https://github.com/JuliaManifolds/ManifoldsBase.jl/blob/65780dc51475b255258dfd35a225f09a0cd2810b/src/decorator_trait.jl#L481

implicitly assumes that the error at hand is a DomainError (or one of the other simple errors that has msg and val). So this is nothing to fix here, but maybe to rethink in ManifoldsBase. I am also not sure when this might occur, since for now I can not reproduce this on my machine. Here I for now tried to raise the tolerance instead.

edit: Ah I see if error is none the string is still build before returning false, so that might indeed happen, still – the best would be to build a string s in these two places we can warn/info about that is a bit less dependent on the internals of the error at hand.

edit: Ah we could change both lines and any other line with such access to use showerror instead. This is however still an issue for ManifoldsBase then.

[kellertuer]

Great! We have one false-positive, and now just need to write tests for the checks (one SPD matrix that is not multinomial, one actual point on M); similarly for the tangent space.

Maybe we could also improve the errors that are returned, since for now they would only display the wrong Manifold (i.e. that the point is not on the SPD manifold), which might be misleading.

[kellertuer]

For multinomial SPD I now wrote the tests – and had an idea for random tangents as well, we basically need tangent vectors to multinomial (rows & columns sum to zero) that are symmetric, so we could just pass that to Multinomial and symmetrize afterwards.

edit: ah no, that does not work. Then I do not have a good idea how to generate tangents on multinomalspd

[kellertuer]

Then I would also consider this to be ready for review and merge.

[kellertuer]

The one line we do not hit is a false-negative (sadly).

[mateuszbaran]

Just a few minor comments.

[kellertuer]

This one is now merged with #700, so we just have to wait for the tests.