Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is base_manifold supposed to commute with ProductManifold? #738

Closed
olivierverdier opened this issue Aug 23, 2024 · 4 comments
Closed

Is base_manifold supposed to commute with ProductManifold? #738

olivierverdier opened this issue Aug 23, 2024 · 4 comments

Comments

@olivierverdier
Copy link
Contributor

It is currently not the case, but perhaps it is supposed to work that way?
Example:

G = SpecialOrthogonal(3)
M = ConnectionManifold(G, CartanSchoutenZero())
base_manifold(M) # Rotations(3)

MM = ProductManifold(M, M)
base_manifold(MM) # MM, but I would have expected ProductManifold(Rotations(3), Rotations(3))
@kellertuer
Copy link
Member

kellertuer commented Aug 23, 2024

Uh, interesting!

For now, the idea is, that it “undecorates”

  • it gives you the manifold of a tangent space
  • it gives you the base manifold if you have a metric decorator, the validation manifold (that performs checks), ...
  • if it not decorated it is of course its own base manifold
    In that sense the Lie group is also decorated and the base is the plain manifold without the Lie structure.

So for now the answer to the question is: No.

I think it could permute, but while it is a philosophical question whether it should (or one of: does someone need it and we provide it in the package for that reason), the larger problem might be the realisation,
since this would have to work for all decorators and recursively? I mean you could do (× is implemented associative and would create a 4-factor product manifold ... but:)

ProductManifold(ProductManifold(ProductManifold(M,M), M), M)

for your case, which would require a (careful) recursive base manifold case.

I personally do not have a strong opinion here. It should be done very carefully and would be a breaking change.

@kellertuer
Copy link
Member

Since base_manifold is really more this technical “strip decorators” idea – maybe a new function like plain_manifold would be the thing you look for? One could then even introduce which features to “strip” (maybe keep a metric?)

That way the technical ‘“undecorate” action would stay as is (we do not have to care about all side effects changing this function would introduce) and we have a “more high level” function doing these more fine granular (and commuting) things.

@olivierverdier
Copy link
Contributor Author

Yes, I think I will use this implementation in my code, this will suffice for my needs, I think.

plain_manifold(M::ProductManifold) = ProductManifold(plain_manifold.(M.manifolds)...)
plain_manifold(M) = base_manifold(M)

@kellertuer
Copy link
Member

An alternative could be to use a keyword argument with the base_manifold like recursive to add your case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants