Sage: Ticket #27442: Implement option for factoring differentials out of Weyl algebras
https://trac.sagemath.org/ticket/27442
<p>
It is natural to write the elements of the (differential) Weyl algebra with the differentials factored out (on the right). This is a common expression for the elements in the single variable case.
</p>
<p>
With this branch
</p>
<pre class="wiki">sage: R.<t> = QQ[]
sage: D = DifferentialWeylAlgebra(R)
sage: t,dt = D.gens()
sage: x = dt^3*t^3 + dt^2*t^4
sage: x
t^3*dt^3 + t^4*dt^2 + 9*t^2*dt^2 + 8*t^3*dt + 18*t*dt + 12*t^2 + 6
sage: D.options.factor_representation = True
sage: x
(12*t^2 + 6) + (8*t^3 + 18*t)dt^1 + (t^4 + 9*t^2)dt^2 + (t^3)dt^3
</pre>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/27442
Trac 1.2Travis ScrimshawFri, 08 Mar 2019 04:49:06 GMTstatus, description changed; commit, branch set
https://trac.sagemath.org/ticket/27442#comment:1
https://trac.sagemath.org/ticket/27442#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>2c071483f74b464bf96849be811814fdffa27f4d</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/27442?action=diff&version=1">diff</a>)
</li>
<li><strong>branch</strong>
set to <em>public/algebras/display_options_weyl-27442</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=2c071483f74b464bf96849be811814fdffa27f4d"><span class="icon"></span>2c07148</a></td><td><code>Adding factored representations to Weyl algebra elements.</code>
</td></tr></table>
TicketTravis ScrimshawMon, 11 Mar 2019 22:36:47 GMTcc set
https://trac.sagemath.org/ticket/27442#comment:2
https://trac.sagemath.org/ticket/27442#comment:2
<ul>
<li><strong>cc</strong>
<em>Frédéric Chapoton</em> added
</li>
</ul>
<p>
I would appreciate a review here (bot is morally green), but I will understand if you do not want to as this is not a trivial ticket.
</p>
TicketErik BrayMon, 25 Mar 2019 10:56:15 GMTmilestone changed
https://trac.sagemath.org/ticket/27442#comment:3
https://trac.sagemath.org/ticket/27442#comment:3
<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>
TicketDaniel KrennWed, 27 Mar 2019 13:55:30 GMT
https://trac.sagemath.org/ticket/27442#comment:4
https://trac.sagemath.org/ticket/27442#comment:4
<p>
A couple of small remarks:
</p>
<ol><li><code>for e,g in</code>: Not sure if PEP8 would say <code>for e, g in</code> (space after comma) here. (found three times)
</li><li><code>[True,False]</code> (very last line of patch): here PEP8 for sure
</li><li>Also <code>R.<x,y,z></code>, <code>x,y,z,dx,dy,dz</code>, <code>t,dt</code> should IMO been written with space after comma.
</li><li><code>factor_differentials</code> is not doctested.
</li><li><code>(8*t^3 + 18*t)dt^1</code> (discussion): Should there be a <code>*</code> before the <code>dt</code>? (In some sense, this would be closer to a representation that one could feed back into the system and let it evaluate (i.e. correct Python syntax). However, I am aware that this might not be a major usecase (if at all).
</li><li>Latex <code>d^{3}_{t}</code>: I simply do not know if this is the standard convention to write to typeset it; I simply believe you here and just wanted it noted.
</li></ol><p>
Otherwise, LGTM.
</p>
TicketDaniel KrennWed, 27 Mar 2019 13:55:43 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/27442#comment:5
https://trac.sagemath.org/ticket/27442#comment:5
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>Daniel Krenn</em>
</li>
</ul>
TicketgitTue, 16 Apr 2019 01:22:35 GMTcommit changed
https://trac.sagemath.org/ticket/27442#comment:6
https://trac.sagemath.org/ticket/27442#comment:6
<ul>
<li><strong>commit</strong>
changed from <em>2c071483f74b464bf96849be811814fdffa27f4d</em> to <em>f3e3321289aa880d76c0ed688d5eec03cad7de3a</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=dc126b76bd814ef1f16e03dbd16fdb760133185e"><span class="icon"></span>dc126b7</a></td><td><code>Merge branch 'public/algebras/display_options_weyl-27442' of git://trac.sagemath.org/sage into public/algebras/display_options_weyl-27442</code>
</td></tr><tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=f3e3321289aa880d76c0ed688d5eec03cad7de3a"><span class="icon"></span>f3e3321</a></td><td><code>Changes from reviewer comments.</code>
</td></tr></table>
TicketgitTue, 16 Apr 2019 01:23:48 GMTcommit changed
https://trac.sagemath.org/ticket/27442#comment:7
https://trac.sagemath.org/ticket/27442#comment:7
<ul>
<li><strong>commit</strong>
changed from <em>f3e3321289aa880d76c0ed688d5eec03cad7de3a</em> to <em>2b0e2be9f1db457169bddcfe3b63491cabe8f819</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=2b0e2be9f1db457169bddcfe3b63491cabe8f819"><span class="icon"></span>2b0e2be</a></td><td><code>Changes from reviewer comments.</code>
</td></tr></table>
TicketTravis ScrimshawTue, 16 Apr 2019 01:27:37 GMT
https://trac.sagemath.org/ticket/27442#comment:8
https://trac.sagemath.org/ticket/27442#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/27442#comment:4" title="Comment 4">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
A couple of small remarks:
</p>
<ol><li><code>for e,g in</code>: Not sure if PEP8 would say <code>for e, g in</code> (space after comma) here. (found three times)
</li></ol></blockquote>
<p>
PEP8 allows you to remove the space for operators when inside of a lower precedence operator. So IMO the no space is fine (and can be more readable). However, I don't care so strongly and have changed it.
</p>
<blockquote class="citation">
<ol start="2"><li><code>[True,False]</code> (very last line of patch): here PEP8 for sure
</li></ol></blockquote>
<p>
See above.
</p>
<blockquote class="citation">
<ol start="2"><li>Also <code>R.<x,y,z></code>, <code>x,y,z,dx,dy,dz</code>, <code>t,dt</code> should IMO been written with space after comma.
</li></ol></blockquote>
<p>
IMO, the <code>R.<x,y,z></code> looks better and is easier to read (and fits PEP8 in the sense given above). However, I agree that there can be spacing for the list of generators, so I have changed those.
</p>
<blockquote class="citation">
<ol start="3"><li><code>factor_differentials</code> is not doctested.
</li></ol></blockquote>
<p>
Whoops, thanks. Fixed.
</p>
<blockquote class="citation">
<ol start="4"><li><code>(8*t^3 + 18*t)dt^1</code> (discussion): Should there be a <code>*</code> before the <code>dt</code>? (In some sense, this would be closer to a representation that one could feed back into the system and let it evaluate (i.e. correct Python syntax). However, I am aware that this might not be a major usecase (if at all).
</li></ol></blockquote>
<p>
That is a good point. I have added that.
</p>
<blockquote class="citation">
<ol start="5"><li>Latex <code>d^{3}_{t}</code>: I simply do not know if this is the standard convention to write to typeset it; I simply believe you here and just wanted it noted.
</li></ol></blockquote>
<p>
It was a good thing to note. In PDEs, people use d<sub>t</sub> for the derivative wrt t. However, in this case, I used the del/<code>\partial</code> notation, so I changed it to be consistent with that.
</p>
TicketDaniel KrennTue, 16 Apr 2019 08:55:32 GMTstatus changed
https://trac.sagemath.org/ticket/27442#comment:9
https://trac.sagemath.org/ticket/27442#comment:9
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/27442#comment:8" title="Comment 8">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/27442#comment:4" title="Comment 4">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
A couple of small remarks:
</p>
<ol><li><code>for e,g in</code>: Not sure if PEP8 would say <code>for e, g in</code> (space after comma) here. (found three times)
</li></ol></blockquote>
<p>
PEP8 allows you to remove the space for operators when inside of a lower precedence operator. So IMO the no space is fine (and can be more readable). However, I don't care so strongly and have changed it.
</p>
</blockquote>
<p>
I see. I know of this rule, but never interpreted it for skipping space after a comma. If I would have known that, I might would have reviewed differently. Anyways, thanks for changing; I think that in many parts of <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>, there are spaces after commas.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ol start="2"><li>Also <code>R.<x,y,z></code>, <code>x,y,z,dx,dy,dz</code>, <code>t,dt</code> should IMO been written with space after comma.
</li></ol></blockquote>
<p>
IMO, the <code>R.<x,y,z></code> looks better and is easier to read (and fits PEP8 in the sense given above). However, I agree that there can be spacing for the list of generators, so I have changed those.
</p>
</blockquote>
<p>
Thanks. LGTM.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ol start="3"><li><code>factor_differentials</code> is not doctested.
</li></ol></blockquote>
<p>
Whoops, thanks. Fixed.
</p>
</blockquote>
<p>
LGTM.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ol start="4"><li><code>(8*t^3 + 18*t)dt^1</code> (discussion): Should there be a <code>*</code> before the <code>dt</code>? (In some sense, this would be closer to a representation that one could feed back into the system and let it evaluate (i.e. correct Python syntax). However, I am aware that this might not be a major usecase (if at all).
</li></ol></blockquote>
<p>
That is a good point. I have added that.
</p>
</blockquote>
<p>
Thanks.
</p>
<blockquote class="citation">
<blockquote class="citation">
<ol start="5"><li>Latex <code>d^{3}_{t}</code>: I simply do not know if this is the standard convention to write to typeset it; I simply believe you here and just wanted it noted.
</li></ol></blockquote>
<p>
It was a good thing to note. In PDEs, people use d<sub>t</sub> for the derivative wrt t. However, in this case, I used the del/<code>\partial</code> notation, so I changed it to be consistent with that.
</p>
</blockquote>
<p>
Ok, thank you for the explanation.
</p>
<p>
So, for me this everything looks fine. Once the patchbot is happy, this is a positive review.
</p>
TicketTravis ScrimshawTue, 16 Apr 2019 11:19:18 GMT
https://trac.sagemath.org/ticket/27442#comment:10
https://trac.sagemath.org/ticket/27442#comment:10
<p>
Thanks. PEP8 allows for some flexibility and interpretation. Bottom line is being consistent and what looks "good".
</p>
<p>
So the patchbot is essentially happy modulo one bad doctest (I swore I tested the file before pushing...), which I will fix when I get to my desktop tomorrow morning (I am based in Australia).
</p>
TicketgitWed, 17 Apr 2019 00:46:05 GMTcommit changed
https://trac.sagemath.org/ticket/27442#comment:11
https://trac.sagemath.org/ticket/27442#comment:11
<ul>
<li><strong>commit</strong>
changed from <em>2b0e2be9f1db457169bddcfe3b63491cabe8f819</em> to <em>cd2a29e0ac1a27c71914484d8ed1bbf30afa8582</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit/?id=cd2a29e0ac1a27c71914484d8ed1bbf30afa8582"><span class="icon"></span>cd2a29e</a></td><td><code>Changes from reviewer comments.</code>
</td></tr></table>
TicketTravis ScrimshawWed, 17 Apr 2019 00:47:58 GMTstatus changed
https://trac.sagemath.org/ticket/27442#comment:12
https://trac.sagemath.org/ticket/27442#comment:12
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
I fixed that one doctest and did a force push since it was a trivial change. With that, I now get the file passing all tests:
</p>
<pre class="wiki">Using --optional=4ti2,coxeter3,dochtml,dot2tex,gambit,latte_int,lidia,lrslib,meataxe,memlimit,mpir,normaliz,p_group_cohomology,pynormaliz,python2,sage,sirocco
Doctesting 1 file using 8 threads.
sage -t --long weyl_algebra.py
[196 tests, 0.16 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.2 seconds
cpu time: 0.2 seconds
cumulative wall time: 0.2 seconds
</pre><p>
This with the previous green bot, I am allowing myself to set a positive review. If you disagree Daniel (and want to wait for another patchbot), just set it back to needs review.
</p>
<p>
Thank you for the review.
</p>
TicketDaniel KrennWed, 17 Apr 2019 07:32:58 GMT
https://trac.sagemath.org/ticket/27442#comment:13
https://trac.sagemath.org/ticket/27442#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/27442#comment:12" title="Comment 12">tscrim</a>:
</p>
<blockquote class="citation">
<p>
I fixed that one doctest and [...]
This with the previous green bot, I am allowing myself to set a positive review. If you disagree Daniel (and want to wait for another patchbot), just set it back to needs review.
</p>
</blockquote>
<p>
Fine for me, thank you.
</p>
<blockquote class="citation">
<p>
Thank you for the review.
</p>
</blockquote>
<p>
You're welcome.
</p>
TicketVolker BraunThu, 18 Apr 2019 19:56:46 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/27442#comment:14
https://trac.sagemath.org/ticket/27442#comment:14
<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/algebras/display_options_weyl-27442</em> to <em>cd2a29e0ac1a27c71914484d8ed1bbf30afa8582</em>
</li>
</ul>
Ticket