Sage: Ticket #25866: Tensor series expansion
https://trac.sagemath.org/ticket/25866
<p>
This ticket modifies the method <code>_simplify</code> of chart functions on manifolds to permit series expansion of tensor field, scalar field or connection.
</p>
<p>
Most of the usual operations are already supported (except Lie derivative).
</p>
<p>
Because the expansion is performed at the lowest level, it provides a major improvement in performances for computing general perturbations (compared with simply expanding after the computation).
</p>
<p>
A notebook example can be found <a class="ext-link" href="http://nbviewer.jupyter.org/github/sagemanifolds/SageManifolds/blob/master/Worksheets/series/First-order%20example.ipynb"><span class="icon"></span>here</a>.
</p>
<p>
This is part of the <a class="ext-link" href="http://sagemanifolds.obspm.fr/"><span class="icon"></span>SageManifolds project</a>; see the metaticket <a class="new ticket" href="https://trac.sagemath.org/ticket/18528" title="task: SageManifolds metaticket 1 (new)">#18528</a> for an overview.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/25866
Trac 1.1.6gh-FlorentinJMon, 06 Aug 2018 09:46:45 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:1
https://trac.sagemath.org/ticket/25866#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketegourgoulhonMon, 10 Sep 2018 10:15:58 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:2
https://trac.sagemath.org/ticket/25866#comment:2
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Thank you very much for having added this new functionality!
The code looks basically good to me. There are some issues with the user interface and documentation though:
</p>
<ul><li>Method <code>ChartFunction.__init__()</code>: the new arguments <code>symbol</code> and <code>order</code> must added in the <code>INPUT</code> section of the documentation of <code>ChartFunction</code>; besides the name <code>symbol</code> is not explicit enough; it should be replaced by something like <code>expansion_symbol</code>
</li><li>Method <code>TensorField.set_calc_order()</code>: no explicit call to that method appears in its doctests!
</li><li>Method <code>TensorFieldParal.series()</code>: it would help the reader if you add the raw output of this method in the doctests, i.e. add
<pre class="wiki"> sage: g.series(e, 3)
[(Field of symmetric bilinear forms on the 4-dimensional Lorentzian manifold M,
0),
(Field of symmetric bilinear forms on the 4-dimensional Lorentzian manifold M,
1),
(Field of symmetric bilinear forms on the 4-dimensional Lorentzian manifold M,
2)]
</pre>before displaying the individual elements of the returned list
</li><li>In the doctests of the same method and as well as in <code>truncate()</code> and <code>set_calc_order()</code>, note that you
can use
<pre class="wiki">sage: g.set(g+e*h1+e**2*h2)
</pre>as a shortcut for
<pre class="wiki">sage: g.set_comp()[:] = (g+e*h1+e**2*h2)[:]
</pre></li><li>Method <code>PseudoRiemannianMetric.inverse()</code>: the name <code>symbol</code> for the argument that triggers the expansion is not explicit enough: it should be changed to something like <code>expand_with_respect_to</code>.
</li><li>Method <code>ScalarField.set_calc_order()</code>: the docstring (probably cut-and-pasted from the tensor field one) must be adapted to the scalar field case; in particular <em>components</em> has no meaning in the current context; besides <em>Tell the ...</em> does not look like a proper phrase for a reference manual...
</li></ul>
TicketegourgoulhonMon, 10 Sep 2018 10:16:28 GMTreviewer set
https://trac.sagemath.org/ticket/25866#comment:3
https://trac.sagemath.org/ticket/25866#comment:3
<ul>
<li><strong>reviewer</strong>
set to <em>Eric Gourgoulhon</em>
</li>
</ul>
TicketgitMon, 08 Oct 2018 00:18:27 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:4
https://trac.sagemath.org/ticket/25866#comment:4
<ul>
<li><strong>commit</strong>
changed from <em>eea81a546948575d7b46870a6bb33780049fce59</em> to <em>154314adada09e3a38a38a73476d7101e8653cc7</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=b20a0417f7be804493fcb245d892de8c2fb51fdb"><span class="icon"></span>b20a041</a></td><td><code>Merge branch 'public/manifolds/series' of git://trac.sagemath.org/sage into public/manifolds/series</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=154314adada09e3a38a38a73476d7101e8653cc7"><span class="icon"></span>154314a</a></td><td><code>Addressing Eric's comments.</code>
</td></tr></table>
TickettscrimMon, 08 Oct 2018 00:19:55 GMT
https://trac.sagemath.org/ticket/25866#comment:5
https://trac.sagemath.org/ticket/25866#comment:5
<p>
This should take care of <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:2" title="Comment 2">comment:2</a> Eric. However, the doctest for <code>set_calc_order</code> in <code>tensorfield.py</code> and <code>tensorfield_paral.py</code> are the same. Florentin, Eric, could one of you get a modified doctest that calls that method specifically?
</p>
TicketgitThu, 25 Oct 2018 09:36:37 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:6
https://trac.sagemath.org/ticket/25866#comment:6
<ul>
<li><strong>commit</strong>
changed from <em>154314adada09e3a38a38a73476d7101e8653cc7</em> to <em>865c5481e0f1ede6aa3ffa85395f56c1ca2b4a86</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=ab5d29211bde9658ab9bd17d5e3d2c5803d8a897"><span class="icon"></span>ab5d292</a></td><td><code>Merge branch 'public/manifolds/series' of git://trac.sagemath.org/sage into series_expansion</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=865c5481e0f1ede6aa3ffa85395f56c1ca2b4a86"><span class="icon"></span>865c548</a></td><td><code>Correct some doctests in series expansions of tensor fields</code>
</td></tr></table>
TicketegourgoulhonThu, 25 Oct 2018 09:41:53 GMT
https://trac.sagemath.org/ticket/25866#comment:7
https://trac.sagemath.org/ticket/25866#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:5" title="Comment 5">tscrim</a>:
</p>
<blockquote class="citation">
<p>
This should take care of <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:2" title="Comment 2">comment:2</a> Eric. However, the doctest for <code>set_calc_order</code> in <code>tensorfield.py</code> and <code>tensorfield_paral.py</code> are the same. Florentin, Eric, could one of you get a modified doctest that calls that method specifically?
</p>
</blockquote>
<p>
Thanks for your changes and apologies for the delay in replying. The above commit fixes some doctests and provides a specific doctest for <code>set_calc_order</code> in <code>tensorfield.py</code>. There are still some documentation issues, that I shall address in a next commit...
</p>
TicketgitThu, 25 Oct 2018 21:25:04 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:8
https://trac.sagemath.org/ticket/25866#comment:8
<ul>
<li><strong>commit</strong>
changed from <em>865c5481e0f1ede6aa3ffa85395f56c1ca2b4a86</em> to <em>75bc8cb90d7029e80ac21addb1a80489428087ec</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=75bc8cb90d7029e80ac21addb1a80489428087ec"><span class="icon"></span>75bc8cb</a></td><td><code>Improvements in the documentation of series expansions of tensor fields</code>
</td></tr></table>
TicketegourgoulhonThu, 25 Oct 2018 21:43:58 GMT
https://trac.sagemath.org/ticket/25866#comment:9
https://trac.sagemath.org/ticket/25866#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:7" title="Comment 7">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
There are still some documentation issues, that I shall address in a next commit...
</p>
</blockquote>
<p>
Done in the above commit.
</p>
<p>
A question remains: the argument <code>order</code> of the various methods <code>set_calc_order</code>, as well as the attribute <code>_order</code>, is the order of the big O. As a result, to get an expansion to first order with respect to the parameter <code>epsilon</code>, we have to do <code>g.set_calc_order(epsilon, 2)</code>. This seems very counter-intuitive and error-prone to me: I would have expected <code>g.set_calc_order(epsilon, 1)</code> instead. I understand that the current setting follows the convention of the method <code>series</code> of symbolic expressions, while I would prefer that it follows the convention of <code>taylor</code>:
</p>
<pre class="wiki">sage: exp(x).series(x, 2)
1 + 1*x + Order(x^2)
sage: exp(x).taylor(x, 0, 1)
x + 1
</pre><p>
(Btw, the above looks like a kind of inconsistency in Sage)
</p>
<p>
What do you think?
</p>
TicketegourgoulhonThu, 25 Oct 2018 21:44:25 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:10
https://trac.sagemath.org/ticket/25866#comment:10
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
TickettscrimFri, 26 Oct 2018 04:18:08 GMT
https://trac.sagemath.org/ticket/25866#comment:11
https://trac.sagemath.org/ticket/25866#comment:11
<p>
I think Sage is not inconsistent as the input to <code>taylor</code> is <code>degree</code>, not <code>order</code>. However, I do agree that there is some ambiguity on which order it is referring too (i.e., first order approximation vs big O order). You could make the case either way. I don't have an opinion either way, but I think both ways are likely to lead to errors. It may be better to call it <code>set_calc_degree</code> to sidestep this issue. Although if you prefer to keep degree, I think either way is fine as long as it is documented with perhaps a <code>.. WARNING::</code> block or two.
</p>
TicketegourgoulhonFri, 26 Oct 2018 14:02:25 GMT
https://trac.sagemath.org/ticket/25866#comment:12
https://trac.sagemath.org/ticket/25866#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:11" title="Comment 11">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I think Sage is not inconsistent as the input to <code>taylor</code> is <code>degree</code>, not <code>order</code>. However, I do agree that there is some ambiguity on which order it is referring too (i.e., first order approximation vs big O order). You could make the case either way. I don't have an opinion either way, but I think both ways are likely to lead to errors.
</p>
</blockquote>
<p>
Well, at least in physics, I would say that the <em>order</em> of an expansion always stands for the degree of the polynomial that approximates the function, not for the degree of the big O. I am pretty sure that this is what the user has in mind when asking for a <em>n</em>-th order computation. Since in the currrent context the big O is not even displayed in the result, I would really favor this interpretation of <code>order</code>.
</p>
<blockquote class="citation">
<p>
It may be better to call it <code>set_calc_degree</code> to sidestep this issue. Although if you prefer to keep degree, I think either way is fine as long as it is documented with perhaps a <code>.. WARNING::</code> block or two.
</p>
</blockquote>
<p>
In the current context, <em>calculation degree</em> sounds awkward to me, contrary to <em>calculation order</em>. Speaking about <code>set_calc_order</code>, are we sure that this is a good name in the first place? This method is indeed the user interface to indicate that one wants to perform an approximate calculation. Would <code>set_series_expansion</code> or <code>use_series_expansion</code> or something else be a better name?
</p>
TickettscrimFri, 26 Oct 2018 17:48:13 GMT
https://trac.sagemath.org/ticket/25866#comment:13
https://trac.sagemath.org/ticket/25866#comment:13
<p>
I don't really are which one we take, but if we use <code>order</code>, it just has to be well documented IMO. I am not sure about <code>series_expansion</code> as that doesn't feel like the correct thing; it is something I would never guess. I think <code>set_calc_order</code> is probably the best we can do; so just documenting what we mean should be enough.
</p>
TicketgitTue, 12 Mar 2019 15:31:55 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:14
https://trac.sagemath.org/ticket/25866#comment:14
<ul>
<li><strong>commit</strong>
changed from <em>75bc8cb90d7029e80ac21addb1a80489428087ec</em> to <em>b8100119f3b9a5f7f7f8b8da6e24e201a7c9241d</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=131ee7d7f8ae2797a3d6e09c8d04c1f6bc0fc62b"><span class="icon"></span>131ee7d</a></td><td><code>Merge branch 'public/manifolds/series' of git://trac.sagemath.org/sage into Sage 8.7.beta7.</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=10356cf2d91040ad1a147c33e724a10ec1961d13"><span class="icon"></span>10356cf</a></td><td><code>Set order to be the degree of the approximating polynomial in tensor series expansions; improve documentation</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=b8100119f3b9a5f7f7f8b8da6e24e201a7c9241d"><span class="icon"></span>b810011</a></td><td><code>Rename method series() to series_expansion() in tensor fields and make it return a list of tensor fields</code>
</td></tr></table>
TicketegourgoulhonTue, 12 Mar 2019 15:50:36 GMT
https://trac.sagemath.org/ticket/25866#comment:15
https://trac.sagemath.org/ticket/25866#comment:15
<p>
Sorry for the long delay in coming back to this!
In the above commits, I have
</p>
<ul><li>set <code>order</code> to be the degree of the polynomial in front of the big O, adding some warnings about this (cf. the discussion in <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:12" title="Comment 12">comment:12</a> and <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:13" title="Comment 13">comment:13</a>)
</li><li>simplified the output of the method <code>TensorFieldParal.series()</code> (there was not need to return a list of pairs with the second element of each pair being merely the index in the list)
</li><li>add the arguments <code>expansion_symbol</code> and <code>order</code> to the method <code>Chart.function()</code> for consistency with the constructor of a chart function
</li><li>improved the documentation, in particular regarding the meaning of <code>order</code>
</li></ul><p>
Besides I've run some benchmarks to check that the new functionalities added by this ticket, which are essentially implemented in <code>ChartFunction._simplify()</code>, do not degrade significantly the tensor calculus performance when no series expansion is required.
</p>
TicketegourgoulhonTue, 12 Mar 2019 15:51:01 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:16
https://trac.sagemath.org/ticket/25866#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
TicketegourgoulhonTue, 12 Mar 2019 15:56:29 GMTmilestone changed
https://trac.sagemath.org/ticket/25866#comment:17
https://trac.sagemath.org/ticket/25866#comment:17
<ul>
<li><strong>milestone</strong>
changed from <em>sage-8.4</em> to <em>sage-8.7</em>
</li>
</ul>
TickettscrimTue, 19 Mar 2019 23:27:50 GMT
https://trac.sagemath.org/ticket/25866#comment:18
https://trac.sagemath.org/ticket/25866#comment:18
<p>
I just have some nitpicks that once changed, you can set a positive review on my behalf.
</p>
<p>
I think this will look at little better in the compiled doc:
</p>
<div class="wiki-code"><div class="code"><pre><span class="gd">-`f_0`, `f_1`, ..., `f_n`
</span><span class="gi">+`f_0, f_1, \ldots, f_n`
</span></pre></div></div><p>
and similarly for the <code>T</code>'s. (Although I am not sure I have actually ever checked this...)
</p>
<p>
In all of the <code>.. MATH::</code> (that are followed by a <code>where</code>), you should add a comma to the end of the formulas for grammatical correctness.
</p>
<p>
I always think of big <code>O</code> with the <code>O</code> appearing as a math symbol, so I would say it should be formatted as <code>big `O`</code> in the doc.
</p>
TickettscrimTue, 19 Mar 2019 23:28:06 GMTreviewer changed
https://trac.sagemath.org/ticket/25866#comment:19
https://trac.sagemath.org/ticket/25866#comment:19
<ul>
<li><strong>reviewer</strong>
changed from <em>Eric Gourgoulhon</em> to <em>Eric Gourgoulhon, Travis Scrimshaw</em>
</li>
</ul>
TicketgitWed, 20 Mar 2019 12:25:49 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:20
https://trac.sagemath.org/ticket/25866#comment:20
<ul>
<li><strong>commit</strong>
changed from <em>b8100119f3b9a5f7f7f8b8da6e24e201a7c9241d</em> to <em>87a256bb56c8e09b6b09550ea2fe1c194b00aa5b</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=87a256bb56c8e09b6b09550ea2fe1c194b00aa5b"><span class="icon"></span>87a256b</a></td><td><code>Improve documentation in series expansion of tensor fields</code>
</td></tr></table>
TickettscrimWed, 20 Mar 2019 14:36:10 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:21
https://trac.sagemath.org/ticket/25866#comment:21
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Thanks. LGTM.
</p>
TicketegourgoulhonWed, 20 Mar 2019 14:48:59 GMT
https://trac.sagemath.org/ticket/25866#comment:22
https://trac.sagemath.org/ticket/25866#comment:22
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:21" title="Comment 21">tscrim</a>:
Thanks for <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:18" title="Comment 18">comment:18</a> and the review!
</p>
TicketegourgoulhonWed, 20 Mar 2019 15:50:55 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:23
https://trac.sagemath.org/ticket/25866#comment:23
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
With the python3 version of Sage 8.7.rc0, building the documentation via
</p>
<pre class="wiki">./sage -docbuild reference/manifolds html
</pre><p>
generates the following warnings:
</p>
<pre class="wiki">[manifolds] <unknown>:2105: DeprecationWarning: invalid escape sequence \e
[manifolds] <unknown>:2243: DeprecationWarning: invalid escape sequence \e
</pre><p>
(there was no such warning with the python2 version).
</p>
<p>
I suspect the issue could be triggered by the <code>\epsilon</code>'s introduced in the documentation. The documentation output looks fine though... Moreover the error message is not very explicit...
</p>
TicketegourgoulhonWed, 20 Mar 2019 16:59:27 GMT
https://trac.sagemath.org/ticket/25866#comment:24
https://trac.sagemath.org/ticket/25866#comment:24
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/25866#comment:23" title="Comment 23">egourgoulhon</a>:
</p>
<blockquote class="citation">
<p>
I suspect the issue could be triggered by the <code>\epsilon</code>'s introduced in the documentation. The documentation output looks fine though... Moreover the error message is not very explicit...
</p>
</blockquote>
<p>
The offending file is <code>src/sage/manifolds/differentiable/tensorfield_paral.py</code> and the numbers 2105 and 2243 in the error message correspond to the ending lines of the docstring of the methods <code>series_expansion</code> and <code>set_calc_order</code> respectively.
I looked through the file and did not find any obvious error... Again the html output is fine (all math formulas involving <code>\epsilon</code> are correctly rendered).
</p>
TicketgitWed, 20 Mar 2019 17:46:05 GMTcommit changed
https://trac.sagemath.org/ticket/25866#comment:25
https://trac.sagemath.org/ticket/25866#comment:25
<ul>
<li><strong>commit</strong>
changed from <em>87a256bb56c8e09b6b09550ea2fe1c194b00aa5b</em> to <em>beaa20e9f75805a8a9a4476195b8aeaee34876aa</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=beaa20e9f75805a8a9a4476195b8aeaee34876aa"><span class="icon"></span>beaa20e</a></td><td><code>Correct docstrings opening in methods series_expansion and set_calc_order of tensorfield_paral.py</code>
</td></tr></table>
TicketegourgoulhonWed, 20 Mar 2019 17:48:05 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:26
https://trac.sagemath.org/ticket/25866#comment:26
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
OK, I found the reason: the docstrings of the two involved methods were starting with <code>"""</code> instead of <code>r"""</code>.
</p>
TickettscrimWed, 20 Mar 2019 22:18:56 GMTstatus changed
https://trac.sagemath.org/ticket/25866#comment:27
https://trac.sagemath.org/ticket/25866#comment:27
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Python3 is very strict about escape sequences, either it is one (which gives a rendering error in Python2) or it raises an error (whereas Python2 silently ignores it).
</p>
TicketembrayMon, 25 Mar 2019 10:56:15 GMTmilestone changed
https://trac.sagemath.org/ticket/25866#comment:28
https://trac.sagemath.org/ticket/25866#comment:28
<ul>
<li><strong>milestone</strong>
changed from <em>sage-8.7</em> to <em>sage-8.8</em>
</li>
</ul>
<p>
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
</p>
TicketvbraunMon, 25 Mar 2019 19:43:59 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/25866#comment:29
https://trac.sagemath.org/ticket/25866#comment:29
<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/series</em> to <em>beaa20e9f75805a8a9a4476195b8aeaee34876aa</em>
</li>
</ul>
Ticket