Sage: Ticket #11273: Riemann Enhancements: Docs, Exterior, Multiple Spiderweb, Error Testing
https://trac.sagemath.org/ticket/11273
<p>
This is a major upgrade of the riemann module. Features are:
</p>
<p>
Fixes and expansions to the docs, covering the issues in <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a>.
</p>
<p>
I've added some test methods that compute the explicit map of an ellipse to a high degree of accuracy. The doctests for these methods contain tests that show that the numerical Riemann_Map values remain accurate. The precision of the normal doctests has been reduced accordingly, so hopefully they will no longer fail for different architectures/numpy versions. This covers <a class="closed ticket" href="https://trac.sagemath.org/ticket/10957" title="defect: Add analytic testing to riemann.pyx (closed: duplicate)">#10957</a>
</p>
<p>
The exterior Riemann map can now be computed, that is a conformal mapping of the exterior of a region to the exterior of the unit circle. The docs specify how this may be done.
</p>
<p>
I've added support for computing the spiderweb plot for multiply connected domains. This uses a different method than the simply connected spiderweb and is takes about the same time as a color plot of the same resolution. (It's worth noting that I don't think this has ever been done before.)
</p>
<p>
I've also fiddled around under the hood, cleaning up some code, moving things around for more efficiency and trying to add a few more helpful inline comments.
</p>
<hr />
<p>
Apply <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Attachment 'trac_11273-rebase-part1.patch' in Ticket #11273">trac_11273-rebase-part1.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Attachment 'trac_11273-rebase-part2.patch' in Ticket #11273">trac_11273-rebase-part2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Attachment 'trac_11273-rebase-part3.patch' in Ticket #11273">trac_11273-rebase-part3.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-reviewer.patch" title="Attachment 'trac_11273-reviewer.patch' in Ticket #11273">trac_11273-reviewer.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-reviewer.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-fix.patch" title="Attachment 'trac_11273-fix.patch' in Ticket #11273">trac_11273-fix.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-fix.patch" title="Download"></a>, and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273_multi_web_fix-review.patch" title="Attachment 'trac_11273_multi_web_fix-review.patch' in Ticket #11273">trac_11273_multi_web_fix-review.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273_multi_web_fix-review.patch" title="Download"></a>.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11273
Trac 1.1.6evanandelThu, 28 Apr 2011 19:06:32 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac-11273-riemann-upgrade-part1.patch</em>
</li>
</ul>
TicketevanandelThu, 28 Apr 2011 19:06:41 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac-11273-riemann-upgrade-part2.patch</em>
</li>
</ul>
TicketevanandelThu, 28 Apr 2011 19:09:10 GMTstatus changed
https://trac.sagemath.org/ticket/11273#comment:1
https://trac.sagemath.org/ticket/11273#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
Because of the complexity of these changes, I decided I didn't want to deal with separating each batch out into its own patch. Hopefully that is acceptable.
</p>
<p>
As a result, this ticket should replace <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a> (some of the other issues there have already been addressed, this just hits the documentation) and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10957" title="defect: Add analytic testing to riemann.pyx (closed: duplicate)">#10957</a>. Is there a good way to go about doing that?
</p>
<p>
Also, as a side note, let me know if there are any places where you would like to see more documentation or tests.
</p>
TicketburcinTue, 31 May 2011 18:29:34 GMTdependencies changed
https://trac.sagemath.org/ticket/11273#comment:2
https://trac.sagemath.org/ticket/11273#comment:2
<ul>
<li><strong>dependencies</strong>
changed from <em>8867, 10792, 10821, 11028</em> to <em>#8867, #10792, #10821, #11028</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11273#comment:1" title="Comment 1">evanandel</a>:
</p>
<blockquote class="citation">
<p>
As a result, this ticket should replace <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a> (some of the other issues there have already been addressed, this just hits the documentation) and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10957" title="defect: Add analytic testing to riemann.pyx (closed: duplicate)">#10957</a>. Is there a good way to go about doing that?
</p>
</blockquote>
<p>
We should close those tickets as duplicates of this one.
</p>
<p>
The patch looks good to me. There are a few minor formatting issues:
</p>
<ul><li>lines should be <= 80 characters. The patch contains some lines with very aggressive wrapping, such as
<pre class="wiki"> q_vector = 1 / (
self.pre_q_vector - pt1)
</pre>and some with almost none.
</li><li>There should be an empty line in the documentation after <code>::</code> to start a code block.
</li><li>I would appreciate it if there were more comments in the code to explain what it's doing, but this is not a requirement.
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac-11273-riemann-upgrade-part2.patch" title="Attachment 'trac-11273-riemann-upgrade-part2.patch' in Ticket #11273">attachment:trac-11273-riemann-upgrade-part2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac-11273-riemann-upgrade-part2.patch" title="Download"></a> cuts down the precision required in the doctests to 5 digits in some places. Is this code really so unstable? Would it help to use other functions for numeric approximation, other than those defined in python's <code>math</code> module?
</li></ul><p>
I am leaving this as <code>needs_review</code> since I don't consider this a full review. Somebody more familiar with this code should comment on the changes and check if this ticket does indeed replace <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10957" title="defect: Add analytic testing to riemann.pyx (closed: duplicate)">#10957</a>.
</p>
TicketkcrismanThu, 09 Jun 2011 02:19:45 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/11273#comment:3
https://trac.sagemath.org/ticket/11273#comment:3
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>Burcin Erocal, Karl-Dieter Crisman</em>
</li>
</ul>
<p>
A few other comments.
</p>
<p>
First, ones to make sure it replaces <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a>.
</p>
<ul><li>Presumably the redefinition of <code>I</code> is supposed to demonstrate it works with lots of different complex types. I suggest the following.
<pre class="wiki"> Can work for different types of complex numbers::
sage: m = Riemann_Map([lambda t: e^(I*t) - 0.5*e^(-I*t)], [lambda t: I*e^(I*t) + 0.5*I*e^(-I*t)], 0) # long time (4 sec)
sage: m.riemann_map(0.25 + sqrt(-0.5)) # long time
(0.137514137885...+0.876696023004...j)
+ sage: I = CDF.gen() # long time
sage: m.riemann_map(1.3*I) # long time
(-1.561029396...+0.989694535737...j)
- sage: I = CDF.gen() # long time
sage: m.riemann_map(0.4) # long time
</pre><ul><li>From <a class="closed ticket" href="https://trac.sagemath.org/ticket/8867" title="enhancement: speed up the riemann mapping functionality (closed: fixed)">#8867</a>: "It now properly avoids failing with lambda functions, although it doesn't work optimally for them. I'll add some notes on that in <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a>."
</li></ul></li></ul><p>
Then a few other comments.
</p>
<ul><li>Incorrect indentation in the last couple functions (in addition to the empty lines Burcin mentioned).
</li><li>Some math things could be put in single backticks, like 2*pi.
</li><li>Commit messages should have the Trac ticket number in them, like <em>Trac 11273, major additions"
</em></li><li>Commit message should have a short first line, then the rest of the information.
</li></ul><hr />
<p>
Hope to later do a better review of the content, which I think I will be able to do. This does need some work due to all these formatting etc. issues, though.
</p>
TicketevanandelThu, 21 Jul 2011 17:44:41 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac-11273-riemann_upgrade_part3.patch</em>
</li>
</ul>
<p>
Formatting Fixes
</p>
TicketevanandelThu, 21 Jul 2011 17:50:03 GMTstatus changed
https://trac.sagemath.org/ticket/11273#comment:4
https://trac.sagemath.org/ticket/11273#comment:4
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Alright I've added a patch that should fix all the formatting fixes mentioned.
</p>
<p>
Regarding code comments, given the complexity of the algorithm, it seemed more suitable to me to link to the appropriate mathematical papers than to attempt to fully document the algorithm in the code.
</p>
<p>
Regarding test precision, no the code is not that unstable. It seems stable to about 10 decimal places or so (although not for nearly 0 values). However, being an numerical algorithm, it is only accurate to roughly 5 decimal places for the number of collocation points in the tests. Therefore it seemed appropriate to try and match the tests to that precision so that if future tweaks change the results--maybe even making them more precise--that won't cause the tests to fail unnecessarily.
</p>
TicketkcrismanThu, 18 Aug 2011 14:36:23 GMT
https://trac.sagemath.org/ticket/11273#comment:5
https://trac.sagemath.org/ticket/11273#comment:5
<p>
Here are some more precise remaining comments.
</p>
<ul><li>As text files, you should make sure to include the Trac number in the commit messages of the first two patches. I recommend just opening them as text, adding that, and then uploading them again ("replace existing file of the same name" or whatever). No need to rebase or something like that, that would be silly when this is so easy to fix.
</li><li>The indention of <code>Simple test::</code> and Testing the accuracy of <code>Riemann_Map::</code> is still not correct. Those should be at the same level as the <code>TESTS:</code> block, I think.
</li><li>In order to make sure we actually we right in closing <a class="closed ticket" href="https://trac.sagemath.org/ticket/8867" title="enhancement: speed up the riemann mapping functionality (closed: fixed)">#8867</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/10945" title="defect: Fix lots of minor docs and redundancy for riemann.pyx (closed: duplicate)">#10945</a>, there has to be some mention of lambda functions. You said so yourself :)
</li><li>I still think that the change in 3 needs to be made, or that test doesn't make sense. If I'm wrong, just clarify, no big deal.
</li></ul><p>
As to review thus far, I have no problem with patch 2 as long as it passes tests, and patch 3 is clearly fine except for the issues mentioned above, which should be fixed.
</p>
<p>
So now I just need to make sure that the overhaul and new stuff in patch 1 is right :) Though I trust it is. I'm looking forward to this being in and at this level, even if I don't get to teach complex analysis for a while.
</p>
TicketevanandelFri, 13 Jan 2012 01:01:17 GMTstatus changed
https://trac.sagemath.org/ticket/11273#comment:6
https://trac.sagemath.org/ticket/11273#comment:6
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
I made the necessary changes. However, now when I try to apply even the original patches the process fails giving me
</p>
<pre class="wiki">
sage: f(t) = e^(I*t) - 0.5*e^(-I*t)
sage: fprime(t) = I*e^(I*t) + 0.5*I*e^(-I*t)
sage: isinstance(Riemann_Map([f], [fprime], 0)._repr_(), str) # long time
True
"""
return "A Riemann mapping of a figure to the unit circle."
cdef _generate_theta_array(self):
"""
</pre><p>
I have absolutely no idea what is causing this. I double checked the first hunk and as far as I can tell what is there exactly matches what the patch expects to find. What is the best way to figure out exactly why these hunks are failing to apply?
</p>
TicketjasonFri, 13 Jan 2012 01:29:19 GMT
https://trac.sagemath.org/ticket/11273#comment:7
https://trac.sagemath.org/ticket/11273#comment:7
<blockquote>
<p>
Can you tell us exactly what you are doing to apply the patches and exactly what it is saying?
</p>
</blockquote>
TicketevanandelTue, 17 Jan 2012 01:24:33 GMT
https://trac.sagemath.org/ticket/11273#comment:8
https://trac.sagemath.org/ticket/11273#comment:8
<p>
Apologies. I posted the wrong bit of text above.
</p>
<p>
I am starting from a clean 4.7.2. First I apply the patch for ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/11028" title="enhancement: More Modular ComplexPlot (closed: fixed)">#11028</a>.
</p>
<pre class="wiki">sage: hg_sage.apply("trac-11028-modular-complex-plot.2.3.patch")
cd "/home/ethan/sage/devel/sage" && hg import "/home/ethan/sage/trac-11028-modular-complex-plot.2.3.patch"
applying /home/ethan/sage/trac-11028-modular-complex-plot.2.3.patch
</pre><p>
The patch applies correctly, and the tests run properly.
</p>
<p>
I then try to apply the first patch from this ticket and get the following result:
</p>
<pre class="wiki">sage: hg_sage.apply("trac-11273-riemann-upgrade-part1.patch")
cd "/home/ethan/sage/devel/sage" && hg import "/home/ethan/sage/trac-11273-riemann-upgrade-part1.patch"
applying /home/ethan/sage/trac-11273-riemann-upgrade-part1.patch
patching file sage/calculus/riemann.pyx
Hunk #9 FAILED at 264
Hunk #28 FAILED at 1012
Hunk #29 FAILED at 1051
3 out of 30 hunks FAILED -- saving rejects to file sage/calculus/riemann.pyx.rej
abort: patch failed to apply
</pre><p>
I could not figure out from the .rej file why the patch is failing. There seem to be no incompatibilities in the significant hunks.
</p>
TicketkcrismanFri, 25 May 2012 16:59:36 GMTdependencies changed
https://trac.sagemath.org/ticket/11273#comment:9
https://trac.sagemath.org/ticket/11273#comment:9
<ul>
<li><strong>dependencies</strong>
changed from <em>#8867, #10792, #10821, #11028</em> to <em>#11028</em>
</li>
</ul>
<p>
Updating so as not to have additional dependencies for patchbot.
</p>
<p>
<a class="closed ticket" href="https://trac.sagemath.org/ticket/11028" title="enhancement: More Modular ComplexPlot (closed: fixed)">#11028</a> has positive review, so now I'll start up on this one. First I'll try to rebase if needed.
</p>
TicketkcrismanFri, 25 May 2012 19:08:41 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-rebase-part2.patch</em>
</li>
</ul>
<p>
Rebased to 5.1.beta0
</p>
TicketkcrismanFri, 25 May 2012 19:08:59 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-rebase-part3.2.patch</em>
</li>
</ul>
<p>
Rebased to 5.1.beta0
</p>
TicketkcrismanFri, 25 May 2012 19:10:48 GMTdescription changed
https://trac.sagemath.org/ticket/11273#comment:10
https://trac.sagemath.org/ticket/11273#comment:10
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=10">diff</a>)
</li>
</ul>
<p>
Apply <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Attachment 'trac_11273-rebase-part1.patch' in Ticket #11273">trac_11273-rebase-part1.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Attachment 'trac_11273-rebase-part2.patch' in Ticket #11273">trac_11273-rebase-part2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Download"></a>, and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Attachment 'trac_11273-rebase-part3.patch' in Ticket #11273">trac_11273-rebase-part3.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Download"></a>.
</p>
<hr />
<p>
Current status is having rebased and fixed a few minor things along the way.
</p>
<p>
Need to:
</p>
<ul><li>Actually review the substantive changes, though these look good at first
</li><li>Remove a lot of whitespace
</li></ul>
TicketkcrismanFri, 25 May 2012 23:39:22 GMT
https://trac.sagemath.org/ticket/11273#comment:11
https://trac.sagemath.org/ticket/11273#comment:11
<p>
Okay, Ethan, here come the questions! Notice I'm still in the middle of reviewing, so I intend to fix a lot of little things yet in followup patches.
</p>
<ul><li>You say
<pre class="wiki">The following inputs must all be passed in as named parameters:
</pre>but they don't, since they already are named, not <code>**kwds</code> (I even tried). Are you thinking of putting in extra arguments later or something? You say something similar with the <code>int</code> argument in <code>get_theta_points</code>.
</li><li>There is a problem with the non-simply-connected plots. Not a big one! But for some reason they don't add properly - maybe you put them in the wrong zorder. Check out these two.
<pre class="wiki"> sage: f(t) = e^(I*t)
sage: fprime(t) = I*e^(I*t)
sage: hf(t) = 0.5*e^(-I*t)
sage: hfprime(t) = 0.5*-I*e^(-I*t)
sage: m = Riemann_Map([f, hf], [fprime, hfprime], 0.5 + 0.5*I)
sage: m.plot_colored() + m.plot_spiderweb() # long time
sage: m.plot_spiderweb()+m.plot_colored()
</pre>I have a feeling this is because of the direct use of a <code>ComplexPlot</code> in the "more difficult multiply connected" branch of the <code>plot_spiderweb</code>, when you already are using a <code>ComplexPlot</code> in the <code>plot_colored</code>, but I'm not sure exactly how this happens, and would rather not track it down.
</li></ul>
TicketkcrismanSat, 26 May 2012 00:37:28 GMT
https://trac.sagemath.org/ticket/11273#comment:12
https://trac.sagemath.org/ticket/11273#comment:12
<p>
More. This first one makes it "needs work". Best to just add another patch after my reviewer patch, which is coming after I finish reviewing.
</p>
<ul><li>I see no doctest for the new <code>exterior</code> keyword. This is important new functionality, so we need it. I would add it myself, but
<pre class="wiki">sage: f(t) = e^(I*t)
sage: fprime(t) = I*e^(I*t)
sage: m = Riemann_Map([f], [fprime], 0) # long time (4 sec)
sage: m.plo
m.plot_boundaries m.plot_colored m.plot_spiderweb
sage: m.plot_spiderweb()+m.plot_colored() # looks good
sage: m = Riemann_Map([f], [fprime], 0, exterior=True) # long time (4 sec)
sage: m.plot_spiderweb()+m.plot_colored() # looks weird
</pre>has a small colored square around a <em>black</em> interior regions, but long long spiderweb, and I'm not sure this is intentional. The same thing happens for other ones made exterior. Also, testing the <code>ValueError</code> raised would be good.
</li><li>Why the difference in the initialization between <code>self.exterior</code> and <code>exterior</code>? Sometimes you use one, sometimes the other, and it's confusing.
</li><li>In general a few of the things for <code>exterior</code> are a little confusing. For instance, <code>pre_q_vector</code> seems to be generated in the same way for both <code>exterior=True</code> and <code>False</code>, whereas nearly everything else is inverted through the origin.
</li><li>I don't know if this needs to be done, but there are places where some checking could be done - like <code>y_points = int((ymax-ymin)/ ystep)</code> seems to be asking for trouble if you ended up with zero for some reason, though in practice one would probably have to try hard to do that.
</li><li>You are a little inconsistent in use of your 'global'-ish names like <code>COMPLEX</code> - sometimes you use it, sometimes you don't.
</li><li>This was a little hard to understand.
<pre class="wiki">#b/c the function is analytic, we know it's abs(derivative) is equal in all directions
</pre>It's not clear exactly what this means, as the derivative of a complex-valued function is a complex number. And partial derivatives don't have to be the same in all directions. I feel like I almost get it, but a little clarification of what you meant by this would be helpful.
</li><li>A few nice (pictorial) examples of why the multiply connected spider plots code works would be good; it seems right from the ones I tried, but seeing some examples would be even better.
</li><li>This is probably too much to ask... would it be possible to have the last three functions (for testing the ellipse example) take epsilon as a parameter instead of just .3? This would help me check whether this is really doing the right thing, though I didn't really have any doubts, given the great pictures!
</li></ul><p>
Maybe you should just send me the Involve article off-list, or have Mike B. send it to me (I don't have access to it). Then I'd really feel confident signing off on this. That said, it really looks good, with the exception of a few mathematical things I want more detail on.
</p>
TicketkcrismanSat, 26 May 2012 00:43:43 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/11273#comment:13
https://trac.sagemath.org/ticket/11273#comment:13
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
set to <em>see comments</em>
</li>
</ul>
<p>
One final thing, which John Palmieri may have to help on. There are warnings when building the documentation that I can't figure out.
</p>
<pre class="wiki">/Users/.../sage-5.1.beta0/devel/sage/doc/en/reference/sage/calculus/riemann.rst:9: WARNING: error while formatting signature for sage.calculus.riemann.complex_to_rgb: Could not parse cython argspec
/Users/.../sage-5.1.beta0/devel/sage/doc/en/reference/sage/calculus/riemann.rst:9: WARNING: error while formatting signature for sage.calculus.riemann.complex_to_spiderweb: Could not parse cython argspec
/Users/.../sage-5.1.beta0/devel/sage/doc/en/reference/sage/calculus/riemann.rst:9: WARNING: error while formatting signature for sage.calculus.riemann.get_derivatives: Could not parse cython argspec
</pre><p>
Though looking at some other tickets, I'm wondering whether this is just something our formatting thing isn't ready for...
</p>
TicketkcrismanSat, 26 May 2012 00:43:50 GMTstatus changed
https://trac.sagemath.org/ticket/11273#comment:14
https://trac.sagemath.org/ticket/11273#comment:14
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketkcrismanSat, 26 May 2012 00:45:55 GMTdescription changed
https://trac.sagemath.org/ticket/11273#comment:15
https://trac.sagemath.org/ticket/11273#comment:15
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=15">diff</a>)
</li>
</ul>
<p>
Apply <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Attachment 'trac_11273-rebase-part1.patch' in Ticket #11273">trac_11273-rebase-part1.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part1.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Attachment 'trac_11273-rebase-part2.patch' in Ticket #11273">trac_11273-rebase-part2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part2.patch" title="Download"></a>, <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Attachment 'trac_11273-rebase-part3.patch' in Ticket #11273">trac_11273-rebase-part3.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-rebase-part3.patch" title="Download"></a>, and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/11273/trac_11273-reviewer.patch" title="Attachment 'trac_11273-reviewer.patch' in Ticket #11273">trac_11273-reviewer.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/11273/trac_11273-reviewer.patch" title="Download"></a>.
</p>
TicketevanandelTue, 11 Sep 2012 22:18:58 GMT
https://trac.sagemath.org/ticket/11273#comment:16
https://trac.sagemath.org/ticket/11273#comment:16
<p>
Thanks for the review. I've tried to address the issues you raise.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11273#comment:11" title="Comment 11">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Okay, Ethan, here come the questions! Notice I'm still in the middle of reviewing, so I intend to fix a lot of little things yet in followup patches.
</p>
<ul><li>You say
<pre class="wiki">The following inputs must all be passed in as named parameters:
</pre>but they don't, since they already are named, not <code>**kwds</code> (I even tried). Are you thinking of putting in extra arguments later or something? You say something similar with the <code>int</code> argument in <code>get_theta_points</code>.
</li></ul></blockquote>
<p>
Updated these descriptions to say "may be passed in". Technically the name isn't required, but it seems helpful to advise it for users less familiar with sage arguments.
</p>
<blockquote class="citation">
<ul><li>There is a problem with the non-simply-connected plots. Not a big one! But for some reason they don't add properly - maybe you put them in the wrong zorder. Check out these two.
<pre class="wiki"> sage: f(t) = e^(I*t)
sage: fprime(t) = I*e^(I*t)
sage: hf(t) = 0.5*e^(-I*t)
sage: hfprime(t) = 0.5*-I*e^(-I*t)
sage: m = Riemann_Map([f, hf], [fprime, hfprime], 0.5 + 0.5*I)
sage: m.plot_colored() + m.plot_spiderweb() # long time
sage: m.plot_spiderweb()+m.plot_colored()
</pre>I have a feeling this is because of the direct use of a <code>ComplexPlot</code> in the "more difficult multiply connected" branch of the <code>plot_spiderweb</code>, when you already are using a <code>ComplexPlot</code> in the <code>plot_colored</code>, but I'm not sure exactly how this happens, and would rather not track it down.
</li></ul></blockquote>
<p>
Good catch. The plots don't add because they are both image type plots, rather than the line plot in the simply connected spiderweb case. To get a color+spiderweb plot you need to use the "withcolor=True" flag. I've updated the docs and doctests to accurately reflect this.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11273#comment:12" title="Comment 12">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
More. This first one makes it "needs work". Best to just add another patch after my reviewer patch, which is coming after I finish reviewing.
</p>
<ul><li>I see no doctest for the new <code>exterior</code> keyword. This is important new functionality, so we need it. I would add it myself, but
<pre class="wiki">sage: f(t) = e^(I*t)
sage: fprime(t) = I*e^(I*t)
sage: m = Riemann_Map([f], [fprime], 0) # long time (4 sec)
sage: m.plo
m.plot_boundaries m.plot_colored m.plot_spiderweb
sage: m.plot_spiderweb()+m.plot_colored() # looks good
sage: m = Riemann_Map([f], [fprime], 0, exterior=True) # long time (4 sec)
sage: m.plot_spiderweb()+m.plot_colored() # looks weird
</pre>has a small colored square around a <em>black</em> interior regions, but long long spiderweb, and I'm not sure this is intentional. The same thing happens for other ones made exterior. Also, testing the <code>ValueError</code> raised would be good.
</li></ul></blockquote>
<p>
Added the appropriate doctests. Spiderweb plots don't work for exterior maps and are now checked for appropriately.
</p>
<blockquote class="citation">
<ul><li>Why the difference in the initialization between <code>self.exterior</code> and <code>exterior</code>? Sometimes you use one, sometimes the other, and it's confusing.
</li></ul></blockquote>
<p>
Fixed.
</p>
<blockquote class="citation">
<ul><li>In general a few of the things for <code>exterior</code> are a little confusing. For instance, <code>pre_q_vect.r</code> seems to be generated in the same way for both <code>exterior=True</code> and <code>False</code>, whereas nearly everything else is inverted through the origin.
</li></ul></blockquote>
<p>
Note the pre_q_vector is basically just a portion of self.cps which <em>is</em> adjusted for exterior,
</p>
<blockquote class="citation">
<ul><li>I don't know if this needs to be done, but there are places where some checking could be done - like <code>y_points = int((ymax-ymin)/ ystep)</code> seems to be asking for trouble if you ended up with zero for some reason, though in practice one would probably have to try hard to do that.
</li></ul></blockquote>
<p>
I'm going to pass on this for now. I've tried to catch the less obvious gotchas, but testing all combinations of bad parameters seems like overkill.
</p>
<blockquote class="citation">
<ul><li>You are a little inconsistent in use of your 'global'-ish names like <code>COMPLEX</code> - sometimes you use it, sometimes you don't.
</li><li>This was a little hard to understand.
<pre class="wiki">#b/c the function is analytic, we know it's abs(derivative) is equal in all directions
</pre>It's not clear exactly what this means, as the derivative of a complex-valued function is a complex number. And partial derivatives don't have to be the same in all directions. I feel like I almost get it, but a little clarification of what you meant by this would be helpful.
</li></ul></blockquote>
<p>
Clarified. Let me know if it's still unclear.
</p>
<blockquote class="citation">
<ul><li>A few nice (pictorial) examples of why the multiply connected spider plots code works would be good; it seems right from the ones I tried, but seeing some examples would be even better.
</li></ul></blockquote>
<p>
I'm not sure what you mean by examples showing <em>how</em> it works.
</p>
<blockquote class="citation">
<ul><li>This is probably too much to ask... would it be possible to have the last three functions (for testing the ellipse example) take epsilon as a parameter instead of just .3? This would help me check whether this is really doing the right thing, though I didn't really have any doubts, given the great pictures!
</li></ul></blockquote>
<p>
Done.
</p>
<blockquote class="citation">
<p>
Maybe you should just send me the Involve article off-list, or have Mike B. send it to me (I don't have access to it). Then I'd really feel confident signing off on this. That said, it really looks good, with the exception of a few mathematical things I want more detail on.
</p>
</blockquote>
<p>
Done. Let me know if you didn't get the email.
</p>
TicketevanandelTue, 11 Sep 2012 22:19:28 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-fix.patch</em>
</li>
</ul>
TicketevanandelTue, 11 Sep 2012 22:20:30 GMTstatus, description changed
https://trac.sagemath.org/ticket/11273#comment:17
https://trac.sagemath.org/ticket/11273#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=17">diff</a>)
</li>
</ul>
TicketkcrismanThu, 03 Jan 2013 19:47:47 GMT
https://trac.sagemath.org/ticket/11273#comment:18
https://trac.sagemath.org/ticket/11273#comment:18
<blockquote class="citation">
<p>
Thanks for the review. I've tried to address the issues you raise.
</p>
</blockquote>
<p>
Thanks!
</p>
<blockquote class="citation">
<p>
Done. Let me know if you didn't get the email.
</p>
</blockquote>
<p>
I did get it. Haven't had time to return to this yet - given that I need to read the article yet. But I am hopeful to review this in 2013 ;-)
</p>
TicketjdemeyerSun, 20 Jan 2013 09:31:44 GMTdescription changed; work_issues deleted
https://trac.sagemath.org/ticket/11273#comment:19
https://trac.sagemath.org/ticket/11273#comment:19
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=19">diff</a>)
</li>
<li><strong>work_issues</strong>
<em>see comments</em> deleted
</li>
</ul>
TicketjdemeyerSun, 20 Jan 2013 10:00:54 GMT
https://trac.sagemath.org/ticket/11273#comment:20
https://trac.sagemath.org/ticket/11273#comment:20
<p>
For a particular build of ATLAS, this still gives
</p>
<pre class="wiki">sage -t "devel/sage/sage/calculus/riemann.pyx"
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 675:
sage: m.inverse_riemann_map(0.5 + sqrt(-0.5))
Expected:
(0.406880...+0.3614702...j)
Got:
(0.26609535153324926-0.71596339253951613j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 677:
sage: m.inverse_riemann_map(0.95)
Expected:
(0.486319...-4.90019052...j)
Got:
(-0.47636117038850095-0.31131054266912134j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 679:
sage: m.inverse_riemann_map(0.25 - 0.3*I)
Expected:
(0.1653244...-0.180936...j)
Got:
(-0.22158720103166435+0.057314947441646828j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 682:
sage: m.inverse_riemann_map(np.complex(-0.2, 0.5))
Expected:
(-0.156280...+0.321819...j)
Got:
(0.30335339499315012-0.10111072188371266j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 783:
sage: print data[0][8,1]
Expected:
(-0.0879...+0.9709...j)
Got:
(-0.596032167594-0.809542574347j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 1086:
sage: dr[8,8]
Expected:
0.241...
Got:
0.27207542829155734
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 1088:
sage: dtheta[5,5]
Expected:
5.907...
Got:
5.664338752865179
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 1440:
sage: abs(m.riemann_map(.5)-analytic_interior(.5, 20, .3)) < 10^-6
Expected:
True
Got:
False
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 411:
sage: s(3*pi / 4)
Expected:
0.0012158...
Got:
0.0011753476894319776
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 484:
sage: s(3*pi / 4)
Expected:
1.627660...
Got:
-2.2112992320625007
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 589:
sage: m.riemann_map(0.25 + sqrt(-0.5))
Expected:
(0.137514...+0.876696...j)
Got:
(-0.72646985217696924-0.51090507937918783j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 591:
sage: m.riemann_map(1.3*I)
Expected:
(-1.56...e-05+0.989694...j)
Got:
(-0.70519318141122977-0.76115177043780835j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 594:
sage: m.riemann_map(0.4)
Expected:
(0.73324...+3.2...e-06j)
Got:
(-0.45716531868823951+0.52177153514479202j)
**********************************************************************
File "/home/buildbot/sage-5.6.rc1/devel/sage/sage/calculus/riemann.pyx", line 597:
sage: m.riemann_map(np.complex(-3, 0.0001))
Expected:
(1.405757...e-05+8.06...e-10j)
Got:
(-0.014156297786253731+0.012030907696686667j)
**********************************************************************
</pre><p>
These failures aren't new, but it would be good to fix them.
</p>
TicketfbisseySun, 20 Jan 2013 10:12:20 GMT
https://trac.sagemath.org/ticket/11273#comment:21
https://trac.sagemath.org/ticket/11273#comment:21
<p>
Out of curiosity. You say "a particular build of ATLAS" and "this is not new". So this is reproducible on a particular machine or it crops up every so often?
</p>
TicketjdemeyerSun, 20 Jan 2013 11:35:23 GMT
https://trac.sagemath.org/ticket/11273#comment:22
https://trac.sagemath.org/ticket/11273#comment:22
<p>
See <a class="ext-link" href="https://groups.google.com/forum/?fromgroups#!topic/sage-devel/h5ezVAp-Z2U"><span class="icon"></span>sage-devel</a>.
</p>
<p>
It crops up every so often on sage.math and is "fixed" by rebuilding ATLAS. But since the only failures are in <code>riemann.pyx</code>, I find it unlikely that ATLAS is really the culprit.
</p>
TicketkcrismanTue, 11 Jun 2013 19:10:49 GMT
https://trac.sagemath.org/ticket/11273#comment:23
https://trac.sagemath.org/ticket/11273#comment:23
<p>
Ah, patchbot...
</p>
<p>
Apply trac_11273-rebase-part1.patch, trac_11273-rebase-part2.patch, trac_11273-rebase-part3.patch, trac_11273-reviewer.patch, and trac_11273-fix.patch.
</p>
<p>
Just for my reference, here are the remaining things I noticed and/or I have to do before giving positive review. Probably some of these can go to other tickets at this point. I'm pretty happy with this overall.
</p>
<ul><li>Mysterious
<pre class="wiki">Converts a list of complex numbers xderivs = get_derivatives(z_values, xstep, ystep)[0]to the plottable
</pre></li><li>Typo <code>equallyspaced</code>
</li><li>Still haven't addressed "Presumably the redefinition of I is ..." (new ticket?)
</li><li>Or "It now properly avoids failing with lambda functions" (new ticket?)
</li><li>Need to read papers (will do now)
</li><li>Possibly some formatting of examples
</li><li>And test all examples again by hand to make sure something didn't slip through!
</li></ul>
TicketkcrismanTue, 11 Jun 2013 19:21:05 GMT
https://trac.sagemath.org/ticket/11273#comment:24
https://trac.sagemath.org/ticket/11273#comment:24
<p>
Aargh. Needs rebase due to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11334" title="task: Update numpy to 1.7.0 (closed: fixed)">#11334</a>.
</p>
TicketkcrismanTue, 11 Jun 2013 19:28:54 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-rebase-part1.patch</em>
</li>
</ul>
<p>
Rebased to 5.1.beta0
</p>
TicketkcrismanTue, 11 Jun 2013 19:35:11 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-rebase-part3.patch</em>
</li>
</ul>
<p>
Rebased to 5.1.beta0
</p>
TicketkcrismanTue, 11 Jun 2013 19:38:41 GMT
https://trac.sagemath.org/ticket/11273#comment:25
https://trac.sagemath.org/ticket/11273#comment:25
<p>
Okay, it was some work but doable.
</p>
<p>
Patchbot, apply trac_11273-rebase-part1.patch, trac_11273-rebase-part2.patch, trac_11273-rebase-part3.patch, trac_11273-reviewer.patch, and trac_11273-fix.patch.
</p>
<p>
I'll get to the rest of this tonight, but hopefully soon all ready!
</p>
TicketkcrismanWed, 12 Jun 2013 02:36:25 GMT
https://trac.sagemath.org/ticket/11273#comment:26
https://trac.sagemath.org/ticket/11273#comment:26
<blockquote class="citation">
<p>
Ah, patchbot...
</p>
</blockquote>
<p>
Thank you, patchbot! Green means go.
</p>
<blockquote class="citation">
<ul><li>Mysterious
<pre class="wiki">Converts a list of complex numbers xderivs = get_derivatives(z_values, xstep, ystep)[0]to the plottable
</pre></li><li>Typo <code>equallyspaced</code>
</li></ul></blockquote>
<p>
Both long ago fixed, not sure how I missed this.
</p>
<blockquote class="citation">
<ul><li>Still haven't addressed "Presumably the redefinition of I is ..."
</li></ul></blockquote>
<p>
I will probably make this a friendly revision to the reviewer patch.
</p>
<blockquote class="citation">
<ul><li>Or "It now properly avoids failing with lambda functions" (new ticket?)
</li></ul></blockquote>
<p>
Well, there's at least <em>one</em> example with a lambda function, so let's let that pass...
</p>
TicketkcrismanWed, 12 Jun 2013 03:29:01 GMT
https://trac.sagemath.org/ticket/11273#comment:27
https://trac.sagemath.org/ticket/11273#comment:27
<p>
In general, I'm happy here. I still find this very interesting and useful.
</p>
<p>
Just one thing; I took an example from the paper and did some stuff.
</p>
<pre class="wiki">ps = polygon_spline([(-4,-2),(4,-2),(4,2),(-4,2)])
z1 = lambda t: ps.value(t); z1p = lambda t: ps.derivative(t)
z2(t) = -2+exp(-I*t); z2p(t) = -I*exp(-I*t)
z3(t) = 2+exp(-I*t); z3p(t) = -I*exp(-I*t)
m = Riemann_Map([z1,z2,z3],[z1p,z2p,z3p],0,ncorners=4)
p = m.plot_spiderweb(withcolor=True,plot_points=500)
show(p,axes=false)
</pre><p>
This one has some of the issues mentioned in the thesis in terms of some weirdness of the spiderweb (in particular, that all the correct stuff is there, but some additional webs as well). I've sent the author an email - perhaps we will want to ask people to not use it with domains <em>too</em> far from being simply connected.
</p>
TicketkcrismanWed, 12 Jun 2013 03:30:03 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273-reviewer.patch</em>
</li>
</ul>
TicketkcrismanWed, 12 Jun 2013 03:31:09 GMTstatus changed; dependencies deleted
https://trac.sagemath.org/ticket/11273#comment:28
https://trac.sagemath.org/ticket/11273#comment:28
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
<li><strong>dependencies</strong>
<em>#11028</em> deleted
</li>
</ul>
<p>
Lest the patchbot try too hard... apply trac_11273-rebase-part1.patch, trac_11273-rebase-part2.patch, trac_11273-rebase-part3.patch, trac_11273-reviewer.patch, and trac_11273-fix.patch.
</p>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/11273#comment:29
https://trac.sagemath.org/ticket/11273#comment:29
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketkcrismanFri, 06 Sep 2013 01:15:26 GMT
https://trac.sagemath.org/ticket/11273#comment:30
https://trac.sagemath.org/ticket/11273#comment:30
<p>
Update:
From personal correspondence, looks like the check for those lines not being drawn was somehow removed in the long sequence of patches here. Likely to have a patch relatively soon.
</p>
TicketevanandelWed, 02 Oct 2013 23:05:35 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273_multi_web_fix.patch</em>
</li>
</ul>
<p>
A fix for exterior fuzz in some multiply connected spiderwebs.
</p>
TicketevanandelWed, 02 Oct 2013 23:12:18 GMTstatus, description changed
https://trac.sagemath.org/ticket/11273#comment:31
https://trac.sagemath.org/ticket/11273#comment:31
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=31">diff</a>)
</li>
</ul>
<p>
It seems that for some domains, particularly those with corners, the numerical integration outside the corners is bad enough to cause "bleed" so that the value outside the domain is farther from zero than desirable. This meant that those portions were escaping the normal check to not draw external lines.
</p>
<p>
I've made the cutoff a parameter (min_mag) so that the user can make it higher if they're observing lines outside the domain. The only trade-off is that the lines will stop slightly farther from the "centers" of the domain. There is a doctest illustrating the increased cutoff.
</p>
<p>
At the request of the reviewer in correspondence, I've also added a fun example to the top level doctests.
</p>
TicketkcrismanWed, 02 Oct 2013 23:49:14 GMT
https://trac.sagemath.org/ticket/11273#comment:32
https://trac.sagemath.org/ticket/11273#comment:32
<p>
This will need a little TLC for a couple minor formatting things, but I think this fix makes sense. I'll check it out, thanks!
</p>
TicketkcrismanThu, 03 Oct 2013 00:16:27 GMT
https://trac.sagemath.org/ticket/11273#comment:33
https://trac.sagemath.org/ticket/11273#comment:33
<p>
Okay, this looks great. Thank you so much for finishing these last tweaks. I'll upload the formatting fix version of your latest patch and we'll be good to go!
</p>
TicketkcrismanThu, 03 Oct 2013 00:20:54 GMTattachment set
https://trac.sagemath.org/ticket/11273
https://trac.sagemath.org/ticket/11273
<ul>
<li><strong>attachment</strong>
set to <em>trac_11273_multi_web_fix-review.patch</em>
</li>
</ul>
TicketkcrismanThu, 03 Oct 2013 00:23:09 GMTstatus, description changed
https://trac.sagemath.org/ticket/11273#comment:34
https://trac.sagemath.org/ticket/11273#comment:34
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/11273?action=diff&version=34">diff</a>)
</li>
</ul>
<p>
Positive review!
</p>
<p>
Patchbot, apply trac_11273-rebase-part1.patch, trac_11273-rebase-part2.patch, trac_11273-rebase-part3.patch, trac_11273-reviewer.patch, trac_11273-fix.patch, trac_11273_multi_web_fix-review.patch
</p>
TicketjdemeyerMon, 07 Oct 2013 06:49:23 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/11273#comment:35
https://trac.sagemath.org/ticket/11273#comment:35
<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>merged</strong>
set to <em>sage-5.13.beta0</em>
</li>
</ul>
Ticket