Rename MetricSpaces parent method metric to metric_function, add EuclideanSpace to category of metric spaces

[mkoeppe]

sage: from sage.categories.metric_spaces import MetricSpaces
sage: QQ in MetricSpaces()
True
sage: RR in MetricSpaces()
True
sage: ZZ in MetricSpaces()
True
sage: AA in MetricSpaces()
False
sage: QuadraticField(5) in MetricSpaces()
False
sage: M = Matrix(QQ,[[2,1,0],[1,2,1],[0,1,2]])
sage: V = VectorSpace(QQ,3,inner_product_matrix=M)
sage: V in MetricSpaces()
False
sage: E = EuclideanSpace(3)
sage: E in MetricSpaces()
False

On this ticket, we add (from #30061 comment:44) EuclideanSpace (implementing the distance function by means of the Cartesian chart) to the category.

To do this, we rename the MetricSpaces parent method metric to metric_function.

The other examples from above will be taken care of in #30092 (which adds inner product spaces) and #30219 (real embedded number fields).

CC: @tscrim @egourgoulhon

Component: categories

Author: Eric Gourgoulhon, Matthias Koeppe

Branch/Commit: d785c0d

Reviewer: Matthias Koeppe, Travis Scrimshaw

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

[mkoeppe]

Description changed:

--- 
+++ 
@@ -16,3 +16,5 @@
 sage: V in MetricSpaces()
 ```
 
+also add (from [#30061 comment:44](https://github.com/sagemath/sage/issues/30061#comment:44)) `EuclideanSpace` (implementing the distance function by means of the Cartesian chart) and possibly other manifold classes.
+

[egourgoulhon]

Branch: public/manifolds/euclidean_metric_space

[egourgoulhon]

New commits:

1f4f942

Provide EuclideanSpace with dist() method and add it to category of metric space

[egourgoulhon]

Commit: 1f4f942

[egourgoulhon]

comment:4

The above commit is a first attempt to add EuclideanSpace to the category of metric spaces. It endows the class EuclideanSpace with the method dist(), which computes the Euclidean distance between two points. However, TestSuite(E).run() fails with _test_metric because of the name clash between

• the method metric() of EuclideanSpace, which is inheritated from PseudoRiemannianManifold and which returns the metric tensor (first fundamental form)
• the method metric() of ParentMethods in MetricSpaces, which is supposed to be the distance function.

It seems difficult to change the name of PseudoRiemannianManifold.metric(), even with a proper deprecation notice, because it is heavily used and it is the natural name in the context of differential geometry. On the other side, could the parent method metric() of MetricSpaces be changed to something else? e.g. distance_function? Also, it seems quite redundant with the other parent method dist(). Indeed, by default, the parent method metric() is defined as

        def metric(self):
            return lambda a,b: a.dist(b)

with a.dist(b) being self.parent().dist(self, b), so at the end of the day, if M is a metric space, M.metric()(a,b) ends to M.dist(a,b).

[egourgoulhon]

comment:5

Another remark about MetricSpaces: it defines an element method abs(), which returns the distance to the zero element. For EuclideanSpace, the zero element could be defined as the element of Cartesian coordinates (0,0,...,0), but in general, metric spaces (which are not necessarily algebraic structures) do not have any natural "zero" element, do they?

[egourgoulhon]

comment:6

Regarding the addition of Riemannian manifolds beyond EuclideanSpace to the category MetricSpaces, this seems computationaly difficult. It is true that any connected Riemannian manifold can be turned into a metric space by defining the distance function as the infinimum of the lengths of the paths connecting two points, but the latter is not computable in practice.

[mkoeppe]

comment:7

Replying to @egourgoulhon:

The above commit is a first attempt to add EuclideanSpace to the category of metric spaces. It endows the class EuclideanSpace with the method dist(), which computes the Euclidean distance between two points.

Great!

However, TestSuite(E).run() fails with _test_metric because of the name clash between

• the method metric() of EuclideanSpace, which is inheritated from PseudoRiemannianManifold and which returns the metric tensor (first fundamental form)
• the method metric() of ParentMethods in MetricSpaces, which is supposed to be the distance function.

It seems difficult to change the name of PseudoRiemannianManifold.metric(), even with a proper deprecation notice, because it is heavily used and it is the natural name in the context of differential geometry.

I agree.

On the other side, could the parent method metric() of MetricSpaces be changed to something else?

Overall, this category seems not very well developed, and I would be surprised if it was used much in user code. I think it will be safe to make changes to it (with deprecation).

e.g. distance_function?

Sounds good to me. But perhaps we can just deprecate the parent method metric altogether until a clear need for it arises?

[mkoeppe]

comment:8

Replying to @egourgoulhon:

Another remark about MetricSpaces: it defines an element method abs(), which returns the distance to the zero element. For EuclideanSpace, the zero element could be defined as the element of Cartesian coordinates (0,0,...,0), but in general, metric spaces (which are not necessarily algebraic structures) do not have any natural "zero" element, do they?

I would propose to deprecate this element method, exactly for this reason. Probably when the category was written, the author was thinking of normed spaces; but these should become a separate category.

[mkoeppe]

comment:9

Replying to @egourgoulhon:

Regarding the addition of Riemannian manifolds beyond EuclideanSpace to the category MetricSpaces, this seems computationaly difficult. It is true that any connected Riemannian manifold can be turned into a metric space by defining the distance function as the infinimum of the lengths of the paths connecting two points, but the latter is not computable in practice.

Good point about the needed hypothesis of connectedness. That alone is already a good enough reason not to add Riemannian manifolds to the category. So we can ignore the more subtle question whether it makes sense to add it to the category but have the dist method raise NotImplementedError for nontrivial inputs.

[tscrim]

comment:10

Indeed, I was conflating metric spaces and normed spaces, which should be separated into 2 separate categories (with normed spaces being a subcategory of metric spaces of course).

Just to note: We do have an axiom for connected and the category Manifolds().Connected().

I am -1 for removing the element and parent methods dist and the metric method. a.dist(b) is natural and easier to work with when you don't want to explicitly specify the parent of a (and b). It is also more conceptual when you don't want to care about the exact metric as long as there is some metric. Similarly, when you want to specify which metric (space) you want to use M.dist(a,b) or M.metric()(a,b) becomes the more natural choice. Additionally, sometimes you want to explicitly work or pass the metric, so M.metric() is something useful too. There should not be any conflict here and having extra (natural) methods doesn't hurt anything.

I would also be wary of saying there is no need just because something is not used in the Sage code: It could be used in a user's code and it feels natural from a teaching perspective. Additionally, me being the (dumb) non-geometer that I am, I would expect asking for the metric() of a metric space to return the metric function d: X x X -> R.

As Eric said in comment:4, the real issue is the name conflict. From some wikipedia searching, what is currently called a metric in the manifolds code is really a (very common) shorthand name for "metric tensor," whereas "metric" is the standard name for the function that defines a metric space. Of course, the metric tensor allows one to define a metric as Eric said, and it is clear from the context of the different fields what one means. Unfortunately there is not a good way to resolve this other than possibly replacing metric with metric_tensor in the manifolds code. However, I think it is reasonable to not put the connected Riemannian manifolds in the MetricSpaces category because of the computational infeasibility of the metric function.

[mkoeppe]

comment:11

Just a quick note:

Replying to @tscrim:

I am -1 for removing the element and parent methods dist and the metric method. a.dist(b) is natural and easier to work with when you don't want to explicitly specify the parent of a (and b).

There is no problem with the element method dist. The only problem is the clash of the parent method metric.

[egourgoulhon]

comment:12

Replying to @tscrim:

I would also be wary of saying there is no need just because something is not used in the Sage code: It could be used in a user's code and it feels natural from a teaching perspective. Additionally, me being the (dumb) non-geometer that I am, I would expect asking for the metric() of a metric space to return the metric function d: X x X -> R.

I agree this quite natural.

As Eric said in comment:4, the real issue is the name conflict. From some wikipedia searching, what is currently called a metric in the manifolds code is really a (very common) shorthand name for "metric tensor," whereas "metric" is the standard name for the function that defines a metric space. Of course, the metric tensor allows one to define a metric as Eric said, and it is clear from the context of the different fields what one means.

Indeed, both usages of metric are natural in their respective contexts.

Unfortunately there is not a good way to resolve this other than possibly replacing metric with metric_tensor in the manifolds code.

Another option is to replace metric by metric_function in MetricSpaces. I would favor that one. From a teaching perspective, it is as easy to discover by TAB completion as metric (contrary to distance_function, which I proposed in comment:4). Moreover, the MetricSpaces's metric() is much less used in Sage's code than the manifold one: grep -r "\.metric()" in src/sage returns 5 lines for metric spaces, versus 127 lines for manifolds.

However, I think it is reasonable to not put the connected Riemannian manifolds in the MetricSpaces category because of the computational infeasibility of the metric function.

I agree, but this does not solve the clash issue for EuclideanSpace, since the latter is considered as a Riemannian manifold (with a flat metric tensor).

[mkoeppe]

comment:13

Replying to @egourgoulhon:

Another option is to replace metric by metric_function in MetricSpaces. I would favor that one. From a teaching perspective, it is as easy to discover by TAB completion as metric (contrary to distance_function, which I proposed in comment:4). Moreover, the MetricSpaces's metric() is much less used in Sage's code than the manifold one: grep -r "\.metric()" in src/sage returns 5 lines for metric spaces, versus 127 lines for manifolds.

+1 on metric_function (with deprecation for metric) in MetricSpaces. I think this is a good compromise.

[tscrim]

comment:14

I agree that renaming metric() -> metric_function() and using that internally is good. However, I might also advocate for doing metric() -> metric_tensor() in the manifold code as it is basically just one sed command, as well as providing metric() as a shorthand.

The slightly trickier question would be should we allow MetricSpaces to have metric() be a shorthand whose semantics could change?