Sage: Ticket #30062: Rename MetricSpaces parent method metric to metric_function, add EuclideanSpace to category of metric spaces
https://trac.sagemath.org/ticket/30062
<pre class="wiki">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
</pre><p>
On this ticket, we add (from <a class="ext-link" href="https://trac.sagemath.org/ticket/30061#comment:44"><span class="icon"></span>https://trac.sagemath.org/ticket/30061#comment:44</a>) <code>EuclideanSpace</code> (implementing the distance function by means of the Cartesian chart) to the category.
</p>
<p>
To do this, we rename the <code>MetricSpaces</code> parent method <code>metric</code> to <code>metric_function</code>.
</p>
<p>
The other examples from above will be taken care of in <a class="new ticket" href="https://trac.sagemath.org/ticket/30092" title="enhancement: Categories of normed additive monoids/groups, normed ... (new)">#30092</a> (which adds inner product spaces) and <a class="new ticket" href="https://trac.sagemath.org/ticket/30219" title="enhancement: Category of real embedded number fields (new)">#30219</a> (real embedded number fields).
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/30062
Trac 1.1.6mkoeppeWed, 08 Jul 2020 13:22:11 GMTcc changed
https://trac.sagemath.org/ticket/30062#comment:1
https://trac.sagemath.org/ticket/30062#comment:1
<ul>
<li><strong>cc</strong>
<em>egourgoulhon</em> added
</li>
</ul>
TicketmkoeppeWed, 08 Jul 2020 22:01:51 GMTdescription changed
https://trac.sagemath.org/ticket/30062#comment:2
https://trac.sagemath.org/ticket/30062#comment:2
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/30062?action=diff&version=2">diff</a>)
</li>
</ul>
TicketegourgoulhonSat, 11 Jul 2020 15:57:25 GMTcommit, branch set
https://trac.sagemath.org/ticket/30062#comment:3
https://trac.sagemath.org/ticket/30062#comment:3
<ul>
<li><strong>commit</strong>
set to <em>1f4f9422e393844eb2edbcbd59b8a8b0b4f5daa4</em>
</li>
<li><strong>branch</strong>
set to <em>public/manifolds/euclidean_metric_space</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=1f4f9422e393844eb2edbcbd59b8a8b0b4f5daa4"><span class="icon"></span>1f4f942</a></td><td><code>Provide EuclideanSpace with dist() method and add it to category of metric space</code>
</td></tr></table>
TicketegourgoulhonSat, 11 Jul 2020 16:23:26 GMT
https://trac.sagemath.org/ticket/30062#comment:4
https://trac.sagemath.org/ticket/30062#comment:4
<p>
The above commit is a first attempt to add <code>EuclideanSpace</code> to the category of metric spaces. It endows the class <code>EuclideanSpace</code> with the method <code>dist()</code>, which computes the Euclidean distance between two points. However, <code>TestSuite(E).run()</code> fails with <code>_test_metric</code> because of the name clash between
</p>
<ul><li>the method <code>metric()</code> of <code>EuclideanSpace</code>, which is inheritated from <code>PseudoRiemannianManifold</code> and which returns the metric tensor (first fundamental form)
</li><li>the method <code>metric()</code> of <code>ParentMethods</code> in <code>MetricSpaces</code>, which is supposed to be the distance function.
</li></ul><p>
It seems difficult to change the name of <code>PseudoRiemannianManifold.metric()</code>, 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 <code>metric()</code> of <code>MetricSpaces</code> be changed to something else? e.g. <code>distance_function</code>? Also, it seems quite redundant with the other parent method <code>dist()</code>. Indeed, by default, the parent method <code>metric()</code> is defined as
</p>
<pre class="wiki"> def metric(self):
return lambda a,b: a.dist(b)
</pre><p>
with <code>a.dist(b)</code> being <code>self.parent().dist(self, b)</code>, so at the end of the day, if <code>M</code> is a metric space, <code>M.metric()(a,b)</code> ends to <code>M.dist(a,b)</code>.
</p>
TicketegourgoulhonSat, 11 Jul 2020 16:28:36 GMT
https://trac.sagemath.org/ticket/30062#comment:5
https://trac.sagemath.org/ticket/30062#comment:5
<p>
Another remark about <code>MetricSpaces</code>: it defines an element method <code>abs()</code>, which returns the distance to the zero element. For <code>EuclideanSpace</code>, the zero element could be defined as the element of Cartesian coordinates <code>(0,0,...,0)</code>, but in general, metric spaces (which are not necessarily algebraic structures) do not have any natural "zero" element, do they?
</p>
TicketegourgoulhonSat, 11 Jul 2020 16:35:32 GMT
https://trac.sagemath.org/ticket/30062#comment:6
https://trac.sagemath.org/ticket/30062#comment:6
<p>
Regarding the addition of Riemannian manifolds beyond <code>EuclideanSpace</code> to the category <code>MetricSpaces</code>, 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.
</p>
TicketmkoeppeSat, 11 Jul 2020 17:54:00 GMT
https://trac.sagemath.org/ticket/30062#comment:7
https://trac.sagemath.org/ticket/30062#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:4" title="Comment 4">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
The above commit is a first attempt to add <code>EuclideanSpace</code> to the category of metric spaces. It endows the class <code>EuclideanSpace</code> with the method <code>dist()</code>, which computes the Euclidean distance between two points.
</p>
</blockquote>
<p>
Great!
</p>
<blockquote class="citation">
<p>
However, <code>TestSuite(E).run()</code> fails with <code>_test_metric</code> because of the name clash between
</p>
<ul><li>the method <code>metric()</code> of <code>EuclideanSpace</code>, which is inheritated from <code>PseudoRiemannianManifold</code> and which returns the metric tensor (first fundamental form)
</li><li>the method <code>metric()</code> of <code>ParentMethods</code> in <code>MetricSpaces</code>, which is supposed to be the distance function.
</li></ul><p>
It seems difficult to change the name of <code>PseudoRiemannianManifold.metric()</code>, even with a proper deprecation notice, because it is heavily used and it is the natural name in the context of differential geometry.
</p>
</blockquote>
<p>
I agree.
</p>
<blockquote class="citation">
<p>
On the other side, could the parent method <code>metric()</code> of <code>MetricSpaces</code> be changed to something else?
</p>
</blockquote>
<p>
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).
</p>
<blockquote class="citation">
<p>
e.g. <code>distance_function</code>?
</p>
</blockquote>
<p>
Sounds good to me. But perhaps we can just deprecate the parent method <code>metric</code> altogether until a clear need for it arises?
</p>
TicketmkoeppeSat, 11 Jul 2020 17:56:45 GMT
https://trac.sagemath.org/ticket/30062#comment:8
https://trac.sagemath.org/ticket/30062#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:5" title="Comment 5">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
Another remark about <code>MetricSpaces</code>: it defines an element method <code>abs()</code>, which returns the distance to the zero element. For <code>EuclideanSpace</code>, the zero element could be defined as the element of Cartesian coordinates <code>(0,0,...,0)</code>, but in general, metric spaces (which are not necessarily algebraic structures) do not have any natural "zero" element, do they?
</p>
</blockquote>
<p>
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.
</p>
TicketmkoeppeSat, 11 Jul 2020 18:01:55 GMT
https://trac.sagemath.org/ticket/30062#comment:9
https://trac.sagemath.org/ticket/30062#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:6" title="Comment 6">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
Regarding the addition of Riemannian manifolds beyond <code>EuclideanSpace</code> to the category <code>MetricSpaces</code>, 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.
</p>
</blockquote>
<p>
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 <code>dist</code> method raise <code>NotImplementedError</code> for nontrivial inputs.
</p>
TickettscrimSun, 12 Jul 2020 03:27:42 GMT
https://trac.sagemath.org/ticket/30062#comment:10
https://trac.sagemath.org/ticket/30062#comment:10
<p>
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).
</p>
<p>
Just to note: We do have an axiom for connected and the category <code>Manifolds().Connected()</code>.
</p>
<p>
I am -1 for removing the element and parent methods <code>dist</code> and the <code>metric</code> method. <code>a.dist(b)</code> is natural and easier to work with when you don't want to explicitly specify the parent of <code>a</code> (and <code>b</code>). 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 <code>M.dist(a,b)</code> or <code>M.metric()(a,b)</code> becomes the more natural choice. Additionally, sometimes you want to explicitly work or pass the metric, so <code>M.metric()</code> is something useful too. There should not be any conflict here and having extra (natural) methods doesn't hurt anything.
</p>
<p>
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 <code>metric()</code> of a metric space to return the metric function <code>d: X x X -> R</code>.
</p>
<p>
As Eric said in <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:4" title="Comment 4">comment:4</a>, 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 <code>metric</code> with <code>metric_tensor</code> in the manifolds code. However, I think it is reasonable to not put the connected Riemannian manifolds in the <code>MetricSpaces</code> category because of the computational infeasibility of the metric function.
</p>
TicketmkoeppeSun, 12 Jul 2020 05:07:31 GMT
https://trac.sagemath.org/ticket/30062#comment:11
https://trac.sagemath.org/ticket/30062#comment:11
<p>
Just a quick note:
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:10" title="Comment 10">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I am -1 for removing the element and parent methods <code>dist</code> and the <code>metric</code> method. <code>a.dist(b)</code> is natural and easier to work with when you don't want to explicitly specify the parent of <code>a</code> (and <code>b</code>).
</p>
</blockquote>
<p>
There is no problem with the element method <code>dist</code>. The only problem is the clash of the parent method <code>metric</code>.
</p>
TicketegourgoulhonSun, 12 Jul 2020 12:50:02 GMT
https://trac.sagemath.org/ticket/30062#comment:12
https://trac.sagemath.org/ticket/30062#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:10" title="Comment 10">tscrim</a>:
</p>
<blockquote class="citation">
<p>
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 <code>metric()</code> of a metric space to return the metric function <code>d: X x X -> R</code>.
</p>
</blockquote>
<p>
I agree this quite natural.
</p>
<blockquote class="citation">
<p>
As Eric said in <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:4" title="Comment 4">comment:4</a>, 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.
</p>
</blockquote>
<p>
Indeed, both usages of <em>metric</em> are natural in their respective contexts.
</p>
<blockquote class="citation">
<p>
Unfortunately there is not a good way to resolve this other than possibly replacing <code>metric</code> with <code>metric_tensor</code> in the manifolds code.
</p>
</blockquote>
<p>
Another option is to replace <code>metric</code> by <code>metric_function</code> in <code>MetricSpaces</code>. I would favor that one. From a teaching perspective, it is as easy to discover by TAB completion as <code>metric</code> (contrary to <code>distance_function</code>, which I proposed in <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:4" title="Comment 4">comment:4</a>). Moreover, the <code>MetricSpaces</code>'s <code>metric()</code> is much less used in Sage's code than the manifold one: <code>grep -r "\.metric()"</code> in <code>src/sage</code> returns 5 lines for metric spaces, versus 127 lines for manifolds.
</p>
<blockquote class="citation">
<p>
However, I think it is reasonable to not put the connected Riemannian manifolds in the <code>MetricSpaces</code> category because of the computational infeasibility of the metric function.
</p>
</blockquote>
<p>
I agree, but this does not solve the clash issue for <code>EuclideanSpace</code>, since the latter is considered as a Riemannian manifold (with a flat metric tensor).
</p>
TicketmkoeppeSun, 12 Jul 2020 17:58:17 GMT
https://trac.sagemath.org/ticket/30062#comment:13
https://trac.sagemath.org/ticket/30062#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:12" title="Comment 12">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
Another option is to replace <code>metric</code> by <code>metric_function</code> in <code>MetricSpaces</code>. I would favor that one. From a teaching perspective, it is as easy to discover by TAB completion as <code>metric</code> (contrary to <code>distance_function</code>, which I proposed in <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:4" title="Comment 4">comment:4</a>). Moreover, the <code>MetricSpaces</code>'s <code>metric()</code> is much less used in Sage's code than the manifold one: <code>grep -r "\.metric()"</code> in <code>src/sage</code> returns 5 lines for metric spaces, versus 127 lines for manifolds.
</p>
</blockquote>
<p>
+1 on <code>metric_function</code> (with deprecation for <code>metric</code>) in <code>MetricSpaces</code>. I think this is a good compromise.
</p>
TickettscrimSun, 12 Jul 2020 23:20:39 GMT
https://trac.sagemath.org/ticket/30062#comment:14
https://trac.sagemath.org/ticket/30062#comment:14
<p>
I agree that renaming <code>metric()</code> -> <code>metric_function()</code> and using that internally is good. However, I might also advocate for doing <code>metric()</code> -> <code>metric_tensor()</code> in the manifold code as it is basically just one sed command, as well as providing <code>metric()</code> as a shorthand.
</p>
<p>
The slightly trickier question would be should we allow <code>MetricSpaces</code> to have <code>metric()</code> be a shorthand whose semantics could change?
</p>
TicketgitFri, 17 Jul 2020 07:19:09 GMTcommit changed
https://trac.sagemath.org/ticket/30062#comment:15
https://trac.sagemath.org/ticket/30062#comment:15
<ul>
<li><strong>commit</strong>
changed from <em>1f4f9422e393844eb2edbcbd59b8a8b0b4f5daa4</em> to <em>7a92b60ad53cc25a20ea86e29ef82fb52f2babc7</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=7a92b60ad53cc25a20ea86e29ef82fb52f2babc7"><span class="icon"></span>7a92b60</a></td><td><code>sage.categories.metric_spaces.MetricSpaces: Rename metric to metric_function with deprecation</code>
</td></tr></table>
TicketmkoeppeFri, 17 Jul 2020 19:24:23 GMT
https://trac.sagemath.org/ticket/30062#comment:16
https://trac.sagemath.org/ticket/30062#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:10" title="Comment 10">tscrim</a>:
</p>
<blockquote class="citation">
<p>
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).
</p>
</blockquote>
<p>
Let's fix this in <a class="new ticket" href="https://trac.sagemath.org/ticket/30092" title="enhancement: Categories of normed additive monoids/groups, normed ... (new)">#30092</a>.
</p>
TicketmkoeppeSat, 25 Jul 2020 21:21:29 GMTdescription, summary changed
https://trac.sagemath.org/ticket/30062#comment:17
https://trac.sagemath.org/ticket/30062#comment:17
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/30062?action=diff&version=17">diff</a>)
</li>
<li><strong>summary</strong>
changed from <em>Add to category of metric spaces</em> to <em>Rename MetricSpaces parent method metric to metric_function, add EuclideanSpace to category of metric spaces</em>
</li>
</ul>
TicketmkoeppeSat, 25 Jul 2020 21:23:23 GMTstatus changed; author set
https://trac.sagemath.org/ticket/30062#comment:18
https://trac.sagemath.org/ticket/30062#comment:18
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>author</strong>
set to <em>Eric Gourgoulhon, Matthias Koeppe</em>
</li>
</ul>
TickettscrimSat, 25 Jul 2020 22:44:56 GMTreviewer set
https://trac.sagemath.org/ticket/30062#comment:19
https://trac.sagemath.org/ticket/30062#comment:19
<ul>
<li><strong>reviewer</strong>
set to <em>Travis Scrimshaw</em>
</li>
</ul>
<p>
While I am not fully convinced that we should not have an ambiguous <code>metric</code> method, it is not a strong enough opinion to warrant asking for further discussion. So if a patchbot comes back (morally) green, then positive review.
</p>
TicketmkoeppeSun, 26 Jul 2020 00:17:11 GMT
https://trac.sagemath.org/ticket/30062#comment:20
https://trac.sagemath.org/ticket/30062#comment:20
<p>
Some test failures that look like this
</p>
<pre class="wiki">sage -t --long src/sage/rings/padics/padic_base_leaves.py
**********************************************************************
File "src/sage/rings/padics/padic_base_leaves.py", line 237, in sage.rings.padics.padic_base_leaves.pAdicRingCappedRelative.__init__
Failed example:
R._test_metric(elements = [R.random_element() for i in range(2^3)]) # long time
Exception raised:
Traceback (most recent call last):
File "sage/structure/category_object.pyx", line 838, in sage.structure.category_object.CategoryObject.getattr_from_category (build/cythonized/sage/structure/category_object.c:6954)
return self.__cached_methods[name]
KeyError: '_test_metric'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 707, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/sagemath/sage-9.1/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1131, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.rings.padics.padic_base_leaves.pAdicRingCappedRelative.__init__[5]>", line 1, in <module>
R._test_metric(elements = [R.random_element() for i in range(Integer(2)**Integer(3))]) # long time
File "sage/structure/category_object.pyx", line 832, in sage.structure.category_object.CategoryObject.__getattr__ (build/cythonized/sage/structure/category_object.c:6876)
return self.getattr_from_category(name)
File "sage/structure/category_object.pyx", line 847, in sage.structure.category_object.CategoryObject.getattr_from_category (build/cythonized/sage/structure/category_object.c:7039)
attr = getattr_from_other_class(self, cls, name)
File "sage/cpython/getattr.pyx", line 389, in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:2548)
raise AttributeError(dummy_error_message)
AttributeError: 'pAdicRingCappedRelative_with_category' object has no attribute '_test_metric'
**********************************************************************
</pre><p>
... and some others
</p>
TicketmkoeppeSun, 26 Jul 2020 00:17:17 GMTstatus changed
https://trac.sagemath.org/ticket/30062#comment:21
https://trac.sagemath.org/ticket/30062#comment:21
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketmkoeppeSun, 26 Jul 2020 00:29:26 GMT
https://trac.sagemath.org/ticket/30062#comment:22
https://trac.sagemath.org/ticket/30062#comment:22
<pre class="wiki">sage -t --long src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst", line 31, in doc.en.thematic_tutorials.vector_calculus.vector_calc_advanced
Failed example:
E.category()
Expected:
Category of smooth manifolds over Real Field with 53 bits of precision
Got:
Join of Category of differentiable manifolds over Real Field with 53 bits of precision and Category of metric spaces
</pre><p>
Is the change from "smooth" to "differentiable" expected here?
</p>
TicketmkoeppeSun, 26 Jul 2020 00:29:33 GMTstatus changed
https://trac.sagemath.org/ticket/30062#comment:23
https://trac.sagemath.org/ticket/30062#comment:23
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
TicketgitSun, 26 Jul 2020 00:30:25 GMTcommit changed
https://trac.sagemath.org/ticket/30062#comment:24
https://trac.sagemath.org/ticket/30062#comment:24
<ul>
<li><strong>commit</strong>
changed from <em>7a92b60ad53cc25a20ea86e29ef82fb52f2babc7</em> to <em>d99aedae454ab73e0a587fd661609e6e3046bd4b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=4ec6e62e08648d465b4e64eeb567a4db1f341f7f"><span class="icon"></span>4ec6e62</a></td><td><code>Merge tag '9.2.beta6' into t/30062/public/manifolds/euclidean_metric_space</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=d99aedae454ab73e0a587fd661609e6e3046bd4b"><span class="icon"></span>d99aeda</a></td><td><code>src/sage/rings/padics/padic_base_leaves.py: Update use of _test_metric</code>
</td></tr></table>
TicketgitSun, 26 Jul 2020 00:33:23 GMTcommit changed
https://trac.sagemath.org/ticket/30062#comment:25
https://trac.sagemath.org/ticket/30062#comment:25
<ul>
<li><strong>commit</strong>
changed from <em>d99aedae454ab73e0a587fd661609e6e3046bd4b</em> to <em>a1389d8b139b76a4b284b746e69f845da25a4361</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=93facad6b5da6e8fdfb3f432226a505f556aecc4"><span class="icon"></span>93facad</a></td><td><code>src/sage/structure/category_object.pyx: Update use of _test_metric</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=a1389d8b139b76a4b284b746e69f845da25a4361"><span class="icon"></span>a1389d8</a></td><td><code>src/doc/en/thematic_tutorials/vector_calculus: Update categories in doctest</code>
</td></tr></table>
TicketmkoeppeSun, 26 Jul 2020 00:33:49 GMTstatus changed
https://trac.sagemath.org/ticket/30062#comment:26
https://trac.sagemath.org/ticket/30062#comment:26
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
TickettscrimSun, 26 Jul 2020 02:28:12 GMT
https://trac.sagemath.org/ticket/30062#comment:27
https://trac.sagemath.org/ticket/30062#comment:27
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:22" title="Comment 22">mkoeppe</a>:
</p>
<blockquote class="citation">
<pre class="wiki">sage -t --long src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/vector_calculus/vector_calc_advanced.rst", line 31, in doc.en.thematic_tutorials.vector_calculus.vector_calc_advanced
Failed example:
E.category()
Expected:
Category of smooth manifolds over Real Field with 53 bits of precision
Got:
Join of Category of differentiable manifolds over Real Field with 53 bits of precision and Category of metric spaces
</pre><p>
Is the change from "smooth" to "differentiable" expected here?
</p>
</blockquote>
<p>
You explicitly changed the default category to only be differentiable rather than smooth:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gi">+ if category is None:
+ category = Manifolds(RR).Differentiable() & MetricSpaces()
</span></pre></div></div><p>
See <code>sage.manifolds.differentiable.manifold.DifferentiableManifold.__init__</code>. So I think you need to change
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">- category = Manifolds(RR).Differentiable() & MetricSpaces()
</span><span class="gi">+ category = Manifolds(RR).Smooth() & MetricSpaces()
</span></pre></div></div>
TicketmkoeppeSun, 26 Jul 2020 02:31:57 GMT
https://trac.sagemath.org/ticket/30062#comment:28
https://trac.sagemath.org/ticket/30062#comment:28
<p>
Eric's commit 1f4f9422 did this, so I guess my question is to him whether this change was intentional
</p>
TickettscrimSun, 26 Jul 2020 04:42:16 GMT
https://trac.sagemath.org/ticket/30062#comment:29
https://trac.sagemath.org/ticket/30062#comment:29
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:28" title="Comment 28">mkoeppe</a>:
</p>
<blockquote class="citation">
<p>
Eric's commit 1f4f9422 did this, so I guess my question is to him whether this change was intentional
</p>
</blockquote>
<p>
Ah, sorry, I thought it was on one of your commits.
</p>
TicketegourgoulhonSun, 26 Jul 2020 12:42:12 GMT
https://trac.sagemath.org/ticket/30062#comment:30
https://trac.sagemath.org/ticket/30062#comment:30
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:28" title="Comment 28">mkoeppe</a>:
</p>
<blockquote class="citation">
<p>
Eric's commit 1f4f9422 did this, so I guess my question is to him whether this change was intentional
</p>
</blockquote>
<p>
Sorry it is a mistake from my side; of course, for Euclidean spaces, the category shall be <code>Manifolds(RR).Smooth()</code>.
</p>
TicketegourgoulhonSun, 26 Jul 2020 12:48:48 GMT
https://trac.sagemath.org/ticket/30062#comment:31
https://trac.sagemath.org/ticket/30062#comment:31
<p>
Since we are on it, we could also add that the Euclidean space is in the category of complete metric spaces:
</p>
<pre class="wiki"> category = Manifolds(RR).Smooth() & MetricSpaces().Complete()
</pre><p>
with hopefully some day <code>RR</code> replaced by a better representation of the real field, as proposed in <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/24456" title="enhancement: classes for the fields of complex and real numbers (needs_work)">#24456</a>.
</p>
TicketegourgoulhonSun, 26 Jul 2020 13:54:17 GMT
https://trac.sagemath.org/ticket/30062#comment:32
https://trac.sagemath.org/ticket/30062#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:31" title="Comment 31">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
Since we are on it, we could also add that the Euclidean space is in the category of complete metric spaces:
</p>
<pre class="wiki"> category = Manifolds(RR).Smooth() & MetricSpaces().Complete()
</pre><p>
with hopefully some day <code>RR</code> replaced by a better representation of the real field, as proposed in <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/24456" title="enhancement: classes for the fields of complex and real numbers (needs_work)">#24456</a>.
</p>
</blockquote>
<p>
I'll add a commit implementing this soon (EDIT: this is the commit in <a class="ticket" href="https://trac.sagemath.org/ticket/30062#comment:33" title="Comment 33">comment:33</a>).
</p>
TicketgitSun, 26 Jul 2020 14:10:50 GMTcommit changed
https://trac.sagemath.org/ticket/30062#comment:33
https://trac.sagemath.org/ticket/30062#comment:33
<ul>
<li><strong>commit</strong>
changed from <em>a1389d8b139b76a4b284b746e69f845da25a4361</em> to <em>d785c0d05c92d9ea96cdaa63df9f52824fa03ce3</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=d785c0d05c92d9ea96cdaa63df9f52824fa03ce3"><span class="icon"></span>d785c0d</a></td><td><code>Better category for Euclidean spaces</code>
</td></tr></table>
TicketmkoeppeSun, 26 Jul 2020 17:26:34 GMTreviewer changed
https://trac.sagemath.org/ticket/30062#comment:34
https://trac.sagemath.org/ticket/30062#comment:34
<ul>
<li><strong>reviewer</strong>
changed from <em>Travis Scrimshaw</em> to <em>Matthias Koeppe, Travis Scrimshaw</em>
</li>
</ul>
TicketmkoeppeSun, 26 Jul 2020 17:27:18 GMT
https://trac.sagemath.org/ticket/30062#comment:35
https://trac.sagemath.org/ticket/30062#comment:35
<p>
Looks great to me, thanks very much.
</p>
<p>
Patchbot is green too. (There is one patchbot that is not feeling well, unrelated to this ticket.)
</p>
TickettscrimMon, 27 Jul 2020 03:39:24 GMTstatus changed
https://trac.sagemath.org/ticket/30062#comment:36
https://trac.sagemath.org/ticket/30062#comment:36
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Thank you.
</p>
TicketvbraunSun, 02 Aug 2020 08:20:40 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/30062#comment:37
https://trac.sagemath.org/ticket/30062#comment:37
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>public/manifolds/euclidean_metric_space</em> to <em>d785c0d05c92d9ea96cdaa63df9f52824fa03ce3</em>
</li>
</ul>
Ticket