Random methods

[ferrine]

It would be cool to have random sampling on the manifold. It should have

• divice, dtype specification
• shape specification

[ferrine]

Also adresses #33

[rrkarim]

Raising matrix to a negative half power is required for uniform distribution on a Stiefel manifold. We need to implement appropriate methods for doing it. Maybe another issue is required for this. #41

[ferrine]

I think it is reasonable to leave this as a future work and implement non uniform

[ferrine]

Proposals

1. put it into an attribute of an instance (I do not like it)

manifold = Manifold()
x = manifold.random.normal(shape)
# or
x = manifold.random.normal(*shape)
# inplace verison
x = manifold.random.normal_(x)
# arguments go as **kwargs
x = manifold.random.normal_(x, **kwargs)

2. naming conventions

manifold = Manifold()
x = manifold.random_normal(shape)
# or
x = manifold.random_normal(*shape)
# inplace verison
x = manifold.random_normal_(x)
# arguments go as **kwargs
x = manifold.random_normal_(x, **kwargs)

3. no naming conventions

manifold = Manifold()
x = manifold.normal(shape)
# or
x = manifold.normal(*shape)
# inplace version
x = manifold.normal_(x)
# arguments go as **kwargs
x = manifold.normal_(x, **kwargs)

4. separate module???
   I have no clues of how does it look like

[SomeoneSerge]

So, given the time constraints...
I'd still avoid putting this stuff into Manifold

Option 1

We could think of manifold-agnostic behaviour: e.g., sample tangents and exponentiate:

def rnorm_exp(manifold, shape):

This way we're explicit that the result isn't Gaussian (we'd need to compute exact squared distances for that).

Option 2

An interface

class ManifoldSampler:
    def sample(shape):

with manifold-specific implementation

class PoincareGaussian(ManifoldSampler):

and user-classes taking

def make_manifold_linear(manifold, gaussian_sampler)

Would work for now, though from the beginning what I don't like is that we'd have to check everyhwere that gaussian_sampler is actually bound to that same manifold

[SomeoneSerge]

manifold = Manifold()

By Manifold you mean SomeSpecificManifold or the abstract one?

[ferrine]

By Manifold you mean SomeSpecificManifold or the abstract one?

An instance of some manifold

[rrkarim]

I don't like the idea with interfaces and actually I forgot why putting random into manifold was a bad idea after all? Separate sampler is not feasible at this moment.

[SomeoneSerge]

manifold.random.normal_(x)
Actually, kind of got a point, if we're to work through the grammar of "composing a manifold from modules" (as in, we got a MetaManifold that takes implementations of retractions, transports and, well, samplers as input parameters and constructs a Manifold with given function sets/given namespace).
We could make a grammar of that, it might be even usable and convenient, but it'd be the kind that is hard to judge about except during runtime. Hard on static analysis

[ferrine]

I liked

class PoincareGaussian(ManifoldSampler):

Maybe we can have an api as in torch.distributions

[SomeoneSerge]

So, we got two options and I think both of them are going to involve class PoincareGaussian, the difference being whether we fuse implementation into SpecificManifold during its runtime construction, or whether we pass it as a parameter to the clients

[ferrine]

But the above idea does not solve common problems like initialization, etc

[SomeoneSerge]

Perhaps, we make class PoincareGaussian now and pass it as a parameter, and then later work on ManifoldMeta so as to simplify the life of the user

[ferrine]

Do not get it

[SomeoneSerge]

But the above idea does not solve common problems like initialization, etc

Please elaborate

[ferrine]

torch.nn.init.normal_(tensor)

[SomeoneSerge]

Do not get it

We write PoincareGaussian now, and make layers for now take it as a parameter.
Later we might make decide to make normal() part of PoincareBall and then normal() would just re-use existing PoincareGaussian

[rrkarim]

So what we are sticking to?

[SomeoneSerge]

I forgot why putting random into manifold was a bad idea after all? Separate sampler is not feasible at this moment

TLDR

I don't like java's way of "fat interfaces"; if we throw a huuuge lot of methods into manifold interface, then we make a promise to implement all of these methods for each new kind of manifold that we add in the future. That might not be that easy. For instance, I'm unsure atm if we can just make Gaussian sampler on some manifold of parameterized distributions (whose density would require evaluating squared distance (the integral) on that manifold, and sampling itself -- well, it needs RnD, so it's better to postpone that decision; it's almost always better to postpone any decision)

Now, we could consider one more option.
One thing I want to make sure is that we got a concise and simple "core functionality" Manifold interface whose namespace ain't spoiled with some stuff we might not need or might not be able to provide. So, we could provide additional interfaces like

class ProvidesGaussianSampling:
  def normal(...):
    raise NotImplementedError()
  def normal_(...):
    raise NotImplementedError()

and LinearLayer then could at least assert isinstance(manifold, ProvidesGaussianSampling).
Btw, do python's type annotations allow to mention multiple interfaces?

[SomeoneSerge]

I guess it's smth like

  def __init__(self, manifold: typing.Tuple[Manifold, ProvidesGaussianSampling], ...):
    pass

Except that sometimes we might want to allow switching between Gaussian and Uniform (between anything)

[rrkarim]

I still don't get how we resolve the issue of the ManifoldTensor being unaware of the manifold it is embedded in. Consequently we can't check the type of the manifold in assert isinstance(manifold, ProvidesGaussianSampling). Maybe you can provide a pseudocode of the logic from the user perspective.

[SomeoneSerge]

Ooook, I'm beginning to realize what is the source of mutual misunderstanding.
So, what I'm talking about is that there shouldn't be random_whatever in Manifold but there might be one in PoincareBall.
As per ProvidesGaussianSampling -- the idea is to allow implementing for example a generic ManifoldLinearLayer, parameterized by a manifold, that would use virtual expmap() and normal() methods in forward() and init_params() resp.

Long story short: ok, go on, make normal() in PoincareBall, just not in Manifold and taking as few parameters as possible

[rrkarim]

I wrote couple of things and I don't like it already. For me it is much more intuitive and simpler to just call X.normal_() (X is a ManifoldTensor) and don't care about the type of manifold and the appropriate samplers to call. But implementing this logic requires me to to define normal in Manifold. I understand that it is harder to maintain in future, but still.

[ferrine]

Agree

Long story short: ok, go on, make normal() in PoincareBall, just not in Manifold and taking as few parameters as possible

Let's implement random methods per manifold without any abstract method. These methods are manifold specific and hence can't be generalized

[SomeoneSerge]

These methods are manifold specific and hence can't be generalized

FTFY: they can be; we just won't

[ferrine]

So, what about random_.* method names, and regular_point method for manifolds? Regular point seems to be useful but semantically meaningless, I'd find a better name. As for random methods, I think we should not aim general purpose random method but implement manifold specific ones. If you guys agree, this can be the next one after #78

[SomeoneSerge]

Was thinking about new_empty instead of regular_point. Think it's better but it might not be immediately obvious from the name that new_empty actually does a non-trivial job of constructing a feasible point.

As for randoms, I still expect that at some point a generic Sampler interface appears with manifold specific implementations and also with generic sampler that makes new_empty() and expmaps from it in random directions.

Still not sure about users of these samplers. Well, later