#31781 closed defect (fixed)

Improve handling of metrics on pseudo-Riemannian submanifolds

Reported by: Eric Gourgoulhon Owned by:
Priority: major Milestone: sage-9.5
Component: manifolds Keywords:
Cc: Michael Jung Merged in:
Authors: Eric Gourgoulhon Reviewers: Michael Jung
Report Upstream: N/A Work issues:
Branch: dc15aff (Commits, GitHub, GitLab) Commit: dc15aff72221572912f47c0ac12f6325ee4da3a7
Dependencies: Stopgaps:

Status badges

Description

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))

Change History (25)

comment:1 Changed 19 months ago by Eric Gourgoulhon

Actually, it appears that g is not initialized:

sage: S2 = manifolds.Sphere(2)                                             
sage: g = S2.metric()                                                        
sage: g._restrictions                                           
{}
Last edited 19 months ago by Eric Gourgoulhon (previous) (diff)

comment:2 Changed 19 months ago by Michael Jung

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.

comment:3 Changed 19 months ago by Michael Jung

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().

comment:4 Changed 19 months ago by Michael Jung

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.

comment:5 in reply to:  3 Changed 19 months ago by Eric Gourgoulhon

Replying to gh-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!

comment:6 in reply to:  4 ; Changed 19 months ago by Eric Gourgoulhon

Replying to gh-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.

comment:7 in reply to:  6 Changed 19 months ago by Michael Jung

Replying to egourgoulhon:

Replying to gh-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.

comment:8 Changed 19 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Moving to 9.4, as 9.3 has been released.

comment:9 Changed 16 months ago by Michael Jung

(see comment:11 below)

Last edited 16 months ago by Michael Jung (previous) (diff)

comment:10 Changed 16 months ago by Michael Jung

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

Last edited 16 months ago by Michael Jung (previous) (diff)

comment:11 in reply to:  9 ; Changed 16 months ago by Michael Jung

Replying to gh-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.

Last edited 16 months ago by Michael Jung (previous) (diff)

comment:12 in reply to:  10 Changed 16 months ago by Eric Gourgoulhon

Replying to gh-mjungmath:

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

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

comment:13 in reply to:  11 ; Changed 16 months ago by Eric Gourgoulhon

Replying to gh-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.

comment:14 in reply to:  13 ; Changed 16 months ago by Michael Jung

Replying to egourgoulhon:

Replying to gh-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!

comment:15 in reply to:  14 ; Changed 16 months ago by Eric Gourgoulhon

Replying to gh-mjungmath:

My thought exactly!

Very good. Do you plan to implement this?

comment:16 in reply to:  15 ; Changed 16 months ago by Michael Jung

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.

comment:17 in reply to:  16 Changed 16 months ago by Eric Gourgoulhon

Replying to gh-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.

comment:18 Changed 16 months ago by Eric Gourgoulhon

Authors: Eric Gourgoulhon
Branch: public/manifolds/submanifold_metric-31781
Commit: dc15aff72221572912f47c0ac12f6325ee4da3a7
Status: newneeds_review
Summary: Unusable metric on spheres from the manifold catalogImprove handling of metrics on pseudo-Riemannian submanifolds

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:

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

comment:19 Changed 16 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:20 Changed 16 months ago by Michael Jung

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?

comment:21 in reply to:  20 Changed 16 months ago by Eric Gourgoulhon

Replying to gh-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).

comment:22 Changed 16 months ago by Michael Jung

Status: needs_reviewpositive_review

Alright.

comment:23 Changed 16 months ago by Michael Jung

Thank you for the explanation.

comment:24 Changed 16 months ago by Eric Gourgoulhon

Reviewers: Michael Jung

Thank you for the review!

comment:25 Changed 15 months ago by Volker Braun

Branch: public/manifolds/submanifold_metric-31781dc15aff72221572912f47c0ac12f6325ee4da3a7
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.