Improve handling of metrics on pseudo-Riemannian submanifolds

[egourgoulhon]

In Sage 9.3.rc5, we have

sage: S2 = manifolds.Sphere(2)                                      
sage: g = S2.metric()
sage: g                                    
Riemannian metric g on the 2-sphere S^2 of radius 1 smoothly embedded 
 in the Euclidean space E^3

So far so good, but then

sage: g.display()
...
ValueError: no basis could be found for computing the components
 in the Coordinate frame (A, (d/dtheta,d/dphi))

sage: g.ricci_scalar()
...
ValueError: no basis could be found for computing the components
 in the Coordinate frame (A, (d/dtheta,d/dphi))

CC: @mjungmath

Component: manifolds

Author: Eric Gourgoulhon

Branch/Commit: dc15aff

Reviewer: Michael Jung

Issue created by migration from https://trac.sagemath.org/ticket/31781

[egourgoulhon]

comment:1

Actually, it appears that g is not initialized:

sage: S2 = manifolds.Sphere(2)                                             
sage: g = S2.metric()                                                        
sage: g._restrictions                                           
{}

[mjungmath]

comment:2

Since spheres are implemented as embedded manifolds, they are only endowed with the induced metric from the ambient space.

Thus I'd say this is no bug. In particular it could happen that the user wants to endow spheres with metrics different from the induced one.

I'd say we just mention this in the documentation.

[mjungmath]

comment:3

Wait. We implemented spheres as Riemannian submanifolds which means that S2.metric() should certainly give the induced metric.

But then this is more a bug of PseudoRiemannianSubmanifold where metric() is not delegating to induced_metric().

[mjungmath]

comment:4

As for my first comment, perhaps it is a good idea to add an optional argument such as induced_metric=True or riemannian=True for spheres to choose whether the submanifold should inherit the metric or not.

[egourgoulhon]

comment:5

Replying to @mjungmath:

Wait. We implemented spheres as Riemannian submanifolds which means that S2.metric() should certainly give the induced metric.

Yes!

But then this is more a bug of PseudoRiemannianSubmanifold where metric() is not delegating to induced_metric().

Indeed!

[egourgoulhon]

comment:6

Replying to @mjungmath:

As for my first comment, perhaps it is a good idea to add an optional argument such as induced_metric=True or riemannian=True for spheres to choose whether the submanifold should inherit the metric or not.

Yes, I would vote for riemannian=True, such that if the user chooses riemannian=False, she/he ends up with a pure differentiable manifold, without any predefined metric. Similarly, we could have differentiable=True, so that choosing differentiable=False returns a mere topological manifold.

[mjungmath]

comment:7

Replying to @egourgoulhon:

Replying to @mjungmath:

As for my first comment, perhaps it is a good idea to add an optional argument such as induced_metric=True or riemannian=True for spheres to choose whether the submanifold should inherit the metric or not.

Yes, I would vote for riemannian=True, such that if the user chooses riemannian=False, she/he ends up with a pure differentiable manifold, without any predefined metric. Similarly, we could have differentiable=True, so that choosing differentiable=False returns a mere topological manifold.

On a second thought, #31241 might be a better solution for that.

[mkoeppe]

comment:8

Moving to 9.4, as 9.3 has been released.

[mjungmath]

comment:9

(see comment:11 below)

[mjungmath]

comment:10

Should we instead attack the issue with PseudoRiemannianSubmanifold and it's metric() call?

[mjungmath]

comment:11

Replying to @mjungmath:

I think we should implement this flag. Ticket #31241 just gives a more natural way to implement this. But I am afraid this ticket is not nearly ready. So I would be fine having a tentative implementation to make the above syntax more convenient for the end-user.

This issue is by the way not restricted to spheres. Also Pseudo-Riemannian manifolds can have different metrics. The current implementation even accommodates this. From that perspective, despite what I said earlier, I actually don't see the need for such a flag because this is in full line with the current way of using Pseudo-Riemannian manifolds.

At this point it might also worth to mention that differentiable manifolds with imposed metrics are not seen as Pseudo-Riemannian manifolds. We don't even have a category for Pseudo-Riemannian manifolds.

[egourgoulhon]

comment:12

Replying to @mjungmath:

Should we instead attack the issue with PseudoRiemannianSubmanifold and it's metric() call?

Yes I think so (see comment:13 below).

[egourgoulhon]

comment:13

Replying to @mjungmath:

This issue is by the way not restricted to spheres. Also Pseudo-Riemannian manifolds can have different metrics. The current implementation even accommodates this. From that perspective, despite what I said earlier, I actually don't see the need for such a flag because this is in full line with the current way of using Pseudo-Riemannian manifolds.

In the current implementation, on any PseudoRiemannianManifold,

• metric() returns the default metric
• metric('h') initializes a new metric with name h

To be consistent, on a PseudoRiemannianSubmanifold, we should have the following behavior:

• metric() redirects to induced_metric(), since this is clearly the default metric on a pseudo-Riemannian submanifold
• metric('h') initializes a new metric with name h

To acheive this, we should redefine metric() in the subclass PseudoRiemannianSubmanifold.

[mjungmath]

comment:14

Replying to @egourgoulhon:

Replying to @mjungmath:

This issue is by the way not restricted to spheres. Also Pseudo-Riemannian manifolds can have different metrics. The current implementation even accommodates this. From that perspective, despite what I said earlier, I actually don't see the need for such a flag because this is in full line with the current way of using Pseudo-Riemannian manifolds.

In the current implementation, on any PseudoRiemannianManifold,

• metric() returns the default metric
• metric('h') initializes a new metric with name h

To be consistent, on a PseudoRiemannianSubmanifold, we should have the following behavior:

• metric() redirects to induced_metric(), since this is clearly the default metric on a pseudo-Riemannian submanifold
• metric('h') initializes a new metric with name h

To acheive this, we should redefine metric() in the subclass PseudoRiemannianSubmanifold.

My thought exactly!

[egourgoulhon]

comment:15

Replying to @mjungmath:

My thought exactly!

Very good. Do you plan to implement this?

[mjungmath]

comment:16

Replying to @egourgoulhon:

My thought exactly!

Very good. Do you plan to implement this?

I'd prefer to first work on tickets that are needed for my current project. Time is slipping through. As soon as those are done, I could come back to this one.

[egourgoulhon]

comment:17

Replying to @mjungmath:

I'd prefer to first work on tickets that are needed for my current project. Time is slipping through. As soon as those are done, I could come back to this one.

No problem, I'll implement it soon.

[egourgoulhon]

comment:18

According to the discussion in comment:13 and comment:14, the above branch reimplements the method metric in the subclass PseudoRiemannianSubmanifold of PseudoRiemannianManifold. In preparing it, I've realized that the handling of the metric names was somewhat messy and I've reorganized it. I've also made the default name of the metric of spheres in the manifold catalog to be g instead of gamma, which is more natural, I think.

---

New commits:

dc15aff

Implement PseudoRiemannianSubmanifold.metric() and various related improvements (Trac #31781)

[egourgoulhon]

Branch: public/manifolds/submanifold_metric-31781

[egourgoulhon]

Commit: dc15aff

[egourgoulhon]

Author: Eric Gourgoulhon

[mjungmath]

comment:20

Is there a specific reason for this change?

@@ -98,12 +98,12 @@ class DegenerateManifold(DifferentiableManifold):
     - [DB1996]_
     - [DS2010]_
     """
-    def __init__(self, n, name, metric_name='g', signature=None, base_manifold=None,
-                 diff_degree=infinity, latex_name=None,
+    def __init__(self, n, name, metric_name=None, signature=None,
+                 base_manifold=None, diff_degree=infinity, latex_name=None,
                  metric_latex_name=None, start_index=0, category=None,
                  unique_tag=None):
         r"""
-        Construct a pseudo-Riemannian manifold.
+        Construct a degenerate manifold.
 
         TESTS::
 
@@ -130,7 +130,9 @@ class DegenerateManifold(DifferentiableManifold):
                                         category=category)
         self._metric = None # to be initialized by metric()
         self._metric_signature = signature
-        if not isinstance(metric_name, str):
+        if metric_name is None:
+            metric_name = 'g'
+        elif not isinstance(metric_name, str):
             raise TypeError("{} is not a string".format(metric_name))
         self._metric_name = metric_name
         if metric_latex_name is None:

Other than that, LGTM. What is wrong with the patchbot?

[egourgoulhon]

comment:21

Replying to @mjungmath:

Is there a specific reason for this change?

-    def __init__(self, n, name, metric_name='g', signature=None, base_manifold=None,
-                 diff_degree=infinity, latex_name=None,
+    def __init__(self, n, name, metric_name=None, signature=None,
+                 base_manifold=None, diff_degree=infinity, latex_name=None,

This is to normalize all the manifold __init__'s; this is mandory for constructing degenerate submanifolds, which inherit both from DegenerateManifold and DifferentiableSubmanifold.

         r"""
-        Construct a pseudo-Riemannian manifold.
+        Construct a degenerate manifold.

This corrects a (copy/paste) error in the docstring.

-        if not isinstance(metric_name, str):
+        if metric_name is None:
+            metric_name = 'g'
+        elif not isinstance(metric_name, str):
             raise TypeError("{} is not a string".format(metric_name))
         self._metric_name = metric_name
         if metric_latex_name is None:

This restores g as the default metric name, after the __init__ entry has been normalized with metric_name=None.

Other than that, LGTM. What is wrong with the patchbot?

I don't know. At least one of them gave a green light (the only plugin failure is non_ascii, but that plugin is no longer relevant and should be removed IMHO).

[mjungmath]

comment:22

Alright.

[mjungmath]

comment:23

Thank you for the explanation.

[egourgoulhon]

comment:24

Thank you for the review!

[egourgoulhon]

Reviewer: Michael Jung

[vbraun]

Changed branch from public/manifolds/submanifold_metric-31781 to dc15aff