Sage: Ticket #10519: analytic combinatorics: new code for computing asymptotics for multivariate generating functions
https://trac.sagemath.org/ticket/10519
<p>
This code is a collection of functions designed
to compute asymptotics of Maclaurin coefficients of certain classes of
multivariate generating functions.
</p>
<p>
The main function asymptotics() returns the first <code>N</code> terms of
the asymptotic expansion of the Maclaurin coefficients <code>F_{n\alpha}</code>
of the multivariate meromorphic function <code>F=G/H</code> as <code>n\to\infty</code>.
It assumes that <code>F</code> is holomorphic in a neighborhood of the origin,
that <code>H</code> is a polynomial, and that asymptotics in the direction of
<code>\alpha</code> (a tuple of positive integers) are controlled by smooth
or multiple points.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10519
Trac 1.1.6araichevWed, 22 Dec 2010 22:28:02 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>mgf.sage</em>
</li>
</ul>
TicketaraichevWed, 22 Dec 2010 22:29:39 GMTmilestone changed
https://trac.sagemath.org/ticket/10519#comment:1
https://trac.sagemath.org/ticket/10519#comment:1
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.6.2</em> to <em>sage-4.6.1</em>
</li>
</ul>
TicketrbeezerMon, 27 Dec 2010 07:39:46 GMT
https://trac.sagemath.org/ticket/10519#comment:2
https://trac.sagemath.org/ticket/10519#comment:2
<p>
Hi Alex,
</p>
<p>
I can't address the mathematics on this one, but can give some hints about getting this reviewed.
</p>
<ol><li> You'll want to submit this as a patch - then folks can run the doctests, check the documentation formatting. There is a lot of checking that can't happen easily with a *.sage file. (See below.) Read the "Walk Through" section of the developer guide for help.
</li></ol><ol start="2"><li> Your examples need slightly different formatting, like
</li></ol><pre class="wiki"> EXAMPLES::
sage: input 1
output-1
Something nice to say. ::
sage: input-2
output-2
</pre><p>
Double-colons start verbatim blocks and sometimes print as colons.
</p>
<p>
Sage code samples needs indentation.
</p>
<ol start="3"><li> Sage is mostly object-oriented. "Internal" method names should begin with an underscore, then they won't show in tab-completion etc (Python convention).
</li></ol><p>
All your functions won't be available in teh global namespace (where they could conflict with other). You should find an <strong>object</strong> (like maybe some kind of polynomial, especially if it is already int eh combinatorial library?) and make your functions be methods on that object.
</p>
<p>
Thanks for your contribution - I hope the above is helpful for you.
</p>
<p>
Rob
</p>
TicketaraichevWed, 05 Jan 2011 00:57:57 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:3
https://trac.sagemath.org/ticket/10519#comment:3
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_work</em>
</li>
</ul>
<p>
Question for you experienced Sage developers: how can i better package my code?
</p>
<p>
I think it makes sense to move the major functions such as 'asymptotics' to methods in the class /Applications/sage/devel/sage/sage/rings/polynomial/multi_polynomial.pyx, but what about the subsidiary functions that don't concern polynomials in particular such as 'diff_prod' which calculates derivatives of products and solves equations?
</p>
TicketslabbeWed, 19 Jan 2011 15:26:42 GMT
https://trac.sagemath.org/ticket/10519#comment:4
https://trac.sagemath.org/ticket/10519#comment:4
<blockquote class="citation">
<p>
Question for you experienced Sage developers: how can i better package my code?
</p>
</blockquote>
<p>
Hi, I am currently at Sage Days 28. Right now, there is a discussion about
Analytic combinatorics in Sage and your code was mentionned in the discussion.
I am not an expert of the domain, but I am coding oriented object Python since
some time now. So below are just some of my thoughts to, I hope, help you turn
your set of functions into an oriented object structure.
</p>
<p>
How to structure a bunch of functions into classes? How to find which objects
(python classes) you need? Here is the trick I personaly use. Consider each of
your functions as a question you ask. Then, ask yourself to who are you asking
each of your questions? Answers often gives you a good hint about the objects
you need to implement. EXAMPLE. Suppose I code the function <code></code>determinant<code></code>.
Question : <code></code>To who do I ask the determinant?<code></code>. Answer: <code></code>To a matrix<code></code>.
Hence, <code></code>matrix<code></code> might be a good object (a python class) to implement.
</p>
<p>
You are the best person to answer to these questions. You might have 30
functions in you file, but only two or three different answers to the above
question. Regroup the similar functions together: they will become the methods
of a same class.
</p>
<p>
The sage file you uploaded starts with :
</p>
<pre class="wiki">> This code relates to analytic combinatorics.
> More specifically, it is a collection of functions designed
> to compute asymptotics of Maclaurin coefficients of certain classes of
> multivariate generating functions.
> The main function asymptotics() returns the first `N` terms of
> the asymptotic expansion of the Maclaurin coefficients `F_{n\alpha}`
> of the multivariate meromorphic function `F=G/H` as `n\to\infty`.
> It assumes that `F` is holomorphic in a neighborhood of the origin,
> that `H` is a polynomial, and that asymptotics in the direction of
> `\alpha` (a tuple of positive integers) are controlled by smooth
> or multiple points.
</pre><p>
Reading only these lines, I imagine the following structure:
</p>
<div class="wiki-code"><div class="code"><pre>
<span class="k">class</span> <span class="nc">HolomorphicMultivariateMeromorphicFunction</span><span class="p">(</span><span class="nb">object</span><span class="p">):</span>
<span class="c"># Constructor of the object</span>
<span class="k">def</span> <span class="nf">__init__</span><span class="p">(</span><span class="bp">self</span><span class="p">,</span> F<span class="p">,</span> G<span class="p">):</span>
<span class="c">#stores important information on the object as attributes of self</span>
<span class="bp">self</span><span class="o">.</span>_F <span class="o">=</span> F
<span class="bp">self</span><span class="o">.</span>_G <span class="o">=</span> G
<span class="k">def</span> <span class="nf">maclaurin_coefficients</span><span class="p">(</span><span class="bp">self</span><span class="p">,</span> n<span class="p">,</span> alpha<span class="p">):</span>
<span class="sd">r"""
Return the maclaurin coefficients of self.
INPUT:
- ``alpha`` - tuple of positive integers
OUTPUT:
a python list of the first terms
OR
maybe an object of a class you implement if there exists pertinent
questions to ask to it.
"""</span>
<span class="c">#Do some computations based (I guess) on self._F and self._G</span>
intermediate_result1 <span class="o">=</span> <span class="bp">self</span><span class="o">.</span>some_intermediate_computations_1<span class="p">()</span>
<span class="c">#Do more computations</span>
<span class="k">return</span> something
<span class="k">def</span> <span class="nf">asymptotics</span><span class="p">(</span><span class="bp">self</span><span class="p">,</span> N<span class="p">,</span> alpha<span class="p">):</span>
<span class="sd">r"""
Returns the asymptotics of Maclaurin coefficients.
"""</span>
<span class="c">#Do some computations based (I guess) on self._F and self._G</span>
intermediate_result2 <span class="o">=</span> <span class="bp">self</span><span class="o">.</span>some_intermediate_computations_2<span class="p">()</span>
intermediate_result3 <span class="o">=</span> <span class="bp">self</span><span class="o">.</span>some_intermediate_computations_3<span class="p">()</span>
<span class="k">return</span> something
<span class="c">#put here all the others functions needed to compute the asymptotics</span>
<span class="k">def</span> <span class="nf">some_intermediate_computations_1</span><span class="p">(</span><span class="bp">self</span><span class="p">):</span>
<span class="k">pass</span>
<span class="k">def</span> <span class="nf">some_intermediate_computations_2</span><span class="p">(</span><span class="bp">self</span><span class="p">):</span>
<span class="k">pass</span>
<span class="k">def</span> <span class="nf">some_intermediate_computations_3</span><span class="p">(</span><span class="bp">self</span><span class="p">):</span>
<span class="k">pass</span>
<span class="o">...</span>
</pre></div></div><p>
It also looks like you need some robustness somehow. But I need to know more
information about what means
</p>
<blockquote class="citation">
<p>
that asymptotics in the direction of <code>\alpha</code> (a tuple of positive integers)
are controlled by smooth or multiple points.
</p>
</blockquote>
<p>
to decide whether this is checked at the creation of the object or before
returning the asymptotics. But these hypothesis should be checked somewhere.
</p>
<p>
Hope this helps.
</p>
<p>
Cheers,
</p>
<p>
Sébastien Labbé, Montréal, (but currently at Sage Days 28, Orsay, France)
</p>
Ticketcyril.banderierWed, 19 Jan 2011 17:07:58 GMTcc set
https://trac.sagemath.org/ticket/10519#comment:5
https://trac.sagemath.org/ticket/10519#comment:5
<ul>
<li><strong>cc</strong>
<em>cyril.banderier</em> added
</li>
</ul>
TicketaraichevThu, 20 Jan 2011 00:07:05 GMT
https://trac.sagemath.org/ticket/10519#comment:6
https://trac.sagemath.org/ticket/10519#comment:6
<p>
Hi Sebastien:
</p>
<p>
Thanks very much for your helpful comments. I've been working on rewriting my code in an object-oriented way and will incorporate your suggestions. Some functions in mgf.sage, such as deciding whether a list of polynomials is algebraically dependent, are general purpose, and so i'm trying to put them into existing Sage classes. The others are more specific to asymptotics and deserve their own class similar to the one you outlined above. Robustness is also something i need to improve upon.
</p>
<p>
I'll post my modifications here after i write and test them more.
</p>
<p>
Thanks again.
</p>
<p>
Alex
</p>
TicketaraichevSun, 01 May 2011 23:59:09 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>amgf_alpha_0_5.sage</em>
</li>
</ul>
<p>
New object oriented version
</p>
TicketaraichevFri, 06 May 2011 00:27:25 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>amgf.sage</em>
</li>
</ul>
<p>
Fixed a bug in cohomologous_integrand()
</p>
TicketaraichevFri, 06 May 2011 00:30:51 GMT
https://trac.sagemath.org/ticket/10519#comment:7
https://trac.sagemath.org/ticket/10519#comment:7
<p>
Hi Sebastien et al:
</p>
<p>
I rewrote my code in object-oriented style. Please have a look if you have the time. Do i need to make the file into a patch, or is it useable as is?
</p>
<p>
Thanks for your help.
Alex
</p>
TicketaraichevFri, 06 May 2011 00:31:18 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:8
https://trac.sagemath.org/ticket/10519#comment:8
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketslabbeWed, 01 Jun 2011 20:56:47 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:9
https://trac.sagemath.org/ticket/10519#comment:9
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Cool. I now see that a lot of the functions are now inside a class, which looks better. So, as I said earlier, this is not really my field of study, so I don't know how far I can go on that review. But, I can suggest at least two things:
</p>
<ol><li>It would be better if you post a Mercurial patch instead of a Sage file so that people can apply it and test it. Are you able to do this? Take a look at the <a class="ext-link" href="http://sagemath.org/doc/developer/"><span class="icon"></span>Sage Developper's Guide</a>, where it should be explain how to create a patch.
</li></ol><p>
To add your file to the documentation, you might want to look <a class="ext-link" href="http://sagemath.org/doc/developer/sage_manuals.html#adding-a-new-file"><span class="icon"></span>here</a>.
</p>
<p>
This winter, I gave a <a class="ext-link" href="http://www.liafa.jussieu.fr/~labbe/Sage/how-to-contribute/"><span class="icon"></span>presentation</a> on How to Contribute to Sage. This may helps you to start with Mercurial for instance.
</p>
<p>
Also, right now, I am working on <a class="closed ticket" href="https://trac.sagemath.org/ticket/11379" title="enhancement: Add Quantumino solver to sage/games (closed: fixed)">#11379</a> where I am adding a new file to Sage. You might find it helpfull to see how I proceed.
</p>
<ol start="2"><li>Some the <a class="ext-link" href="http://sagemath.org/doc/developer/conventions.html#python-coding-conventions"><span class="icon"></span>Python convention</a> are not always followed. See also <a class="ext-link" href="http://www.python.org/dev/peps/pep-0008/"><span class="icon"></span>pep-0008</a>. Especially for spaces :
</li></ol><pre class="wiki"> Yes:
i = i + 1
No:
i= i+1
</pre>
TicketslabbeWed, 01 Jun 2011 21:01:51 GMT
https://trac.sagemath.org/ticket/10519#comment:10
https://trac.sagemath.org/ticket/10519#comment:10
<p>
To add a file to a patch, you must do:
</p>
<pre class="wiki">sage -hg add your_file.py
</pre><p>
I was not able to find this information anywhere which is strange...
</p>
<p>
Tell me if you have questions.
</p>
<p>
Sébastien
</p>
TicketaraichevSat, 11 Jun 2011 04:01:13 GMT
https://trac.sagemath.org/ticket/10519#comment:11
https://trac.sagemath.org/ticket/10519#comment:11
<p>
Thanks for your help, Se'bastien. I'm trying to put my Sage file into a patch by following your presentation slides, but am running into difficulties. I cloned Sage and copied my Sage file as 'amgf.sage' into
</p>
<p>
/Applications/sage/devel/sage-araichev/sage/combinat/
</p>
<p>
I rebuilt Sage etc. and made it to step 10 "Test the changes" of your slides. But when i run Sage and test a few commands, Sage doesn't seem to see the <a class="missing wiki">QuasiRationalExpression?</a> class i created in /Applications/sage/devel/sage-araichev/sage/combinat/amgf.sage.
</p>
<p>
I thought this might be because amgf.sage is a Sage file and not a Python file. So i renamed it amgf.py instead and got syntax errors. What's up with that? Do i have to rewrite all my code in Python instead of Sage?
</p>
<p>
Thanks for your attention.
Alex
</p>
TickethivertSat, 11 Jun 2011 07:19:33 GMT
https://trac.sagemath.org/ticket/10519#comment:12
https://trac.sagemath.org/ticket/10519#comment:12
<blockquote>
<p>
Hi Alex,
</p>
</blockquote>
<blockquote class="citation">
<p>
I thought this might be because amgf.sage is a Sage file and not a Python
file. So i renamed it amgf.py instead and got syntax errors. What's up with
that? Do i have to rewrite all my code in Python instead of Sage?
</p>
</blockquote>
<p>
The basic answer is yes, but Rewriting is a big word for what is really
needed. There is little work to do since Sage mostly follows Python syntax.
The two main difference are handling of integer (see
<a href="http://www.sagemath.org/doc/tutorial/afterword.html">http://www.sagemath.org/doc/tutorial/afterword.html</a>), and the necessity to
import what you need.
</p>
<h2 id="Handlingofinteger">Handling of integer</h2>
<ul><li>Notation for exponentiation: In Python <code>**</code> means exponentiation and
<code>^</code> means “xor”.
</li></ul><ul><li>If you need to return an integer for the user, write it return
<code>Integer(1)</code> instead of return 1. In Python 1 is a machine integer
<code>int</code> (32 or 64 bits depending on your machine) and <code>Integer(1)</code> is
a <a class="missing wiki">Sage/Gmp?</a> arbitrary precision integer. Also <code>Integer</code> are much more
powerful than <code>int</code>, for example they know about prime and
factorization.
</li></ul><h2 id="Importingstuff">Importing stuff</h2>
<p>
The second big change is the necessity to import all what you need. More
precisely, each time you use some Sage function, you need to import it at the
beginning of the file. for example if you want to you <code>PolynomialRing</code>,
you need to write
</p>
<pre class="wiki">from sage.rings.polynomial.polynomial_ring_constructor import PolynomialRing
</pre><p>
You can ask Sage where to find <a class="missing wiki">PolynomialRing?</a> using:
</p>
<pre class="wiki">sage: PolynomialRing.__module__
'sage.rings.polynomial.polynomial_ring_constructor'
</pre><p>
This also correspond to the path starting after <code>site-packages</code>
given when you are asking Sage for
<code>PolynomialRing</code> help:
</p>
<pre class="wiki">sage: PolynomialRing?
Type: function
[...]
File: /home/florent/src/Sage/sage/local/lib/python2.6/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py
[...]
</pre><p>
I hope this helps,
</p>
<p>
Florent
</p>
TicketslabbeSat, 11 Jun 2011 12:51:30 GMT
https://trac.sagemath.org/ticket/10519#comment:13
https://trac.sagemath.org/ticket/10519#comment:13
<p>
Just answering as a complement to Florent's answer.
</p>
<blockquote class="citation">
<p>
But when i run Sage and test a few commands, Sage doesn't seem to see the <a class="missing wiki">QuasiRationalExpression?</a> class i created in /Applications/sage/devel/sage-araichev/sage/combinat/amgf.sage.
</p>
</blockquote>
<p>
To test your code in a Sage session, you will first need to import the good class (or function). For instance :
</p>
<pre class="wiki">sage: from sage.combinat.amgf import QuasiRationalExpression
</pre><p>
If you would like <code>QuasiRationalExpression</code> to be already imported once Sage is started, you must add the line <code>from sage.combinat.amgf import QuasiRationalExpression</code> to the file <code>sage/combinat/all.py</code>. Note that it is not "à la mode" to add new stuff to all.py files because Sage is very slow to start because of all the things that are imported. If you think a lot of people will use the code, then it might be defendable.
</p>
<p>
Sébastien
</p>
TicketaraichevSun, 12 Jun 2011 23:07:51 GMT
https://trac.sagemath.org/ticket/10519#comment:14
https://trac.sagemath.org/ticket/10519#comment:14
<p>
Thanks for the tips, Florent and Se'bastien. I'll get to work on converting amgf.sage to amgf.py.
</p>
<p>
Alex
</p>
TicketaraichevSun, 12 Jun 2011 23:17:27 GMT
https://trac.sagemath.org/ticket/10519#comment:15
https://trac.sagemath.org/ticket/10519#comment:15
<p>
Hang on, can i somehow use the Sage pre-parser to automatically convert amgf.sage to amgf.py?
</p>
<p>
Alex
</p>
TicketaraichevMon, 13 Jun 2011 00:02:41 GMT
https://trac.sagemath.org/ticket/10519#comment:16
https://trac.sagemath.org/ticket/10519#comment:16
<p>
Aha! The command 'sage amgf.sage' pre-parses amgf.sage and saves it as amgf.py. Thank you ask.sagemath.org!
</p>
<p>
Alex
</p>
TicketaraichevMon, 13 Jun 2011 21:44:34 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>amgf.py</em>
</li>
</ul>
<p>
Pythonified amgf.sage
</p>
TicketaraichevMon, 13 Jun 2011 21:55:28 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:17
https://trac.sagemath.org/ticket/10519#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketslabbeWed, 15 Jun 2011 10:13:31 GMT
https://trac.sagemath.org/ticket/10519#comment:18
https://trac.sagemath.org/ticket/10519#comment:18
<p>
The patch is empty. I hope you did not lose anything?
</p>
<p>
Sébastien
</p>
TicketaraichevFri, 17 Jun 2011 03:03:45 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519.patch</em>
</li>
</ul>
TicketaraichevFri, 17 Jun 2011 03:07:38 GMT
https://trac.sagemath.org/ticket/10519#comment:19
https://trac.sagemath.org/ticket/10519#comment:19
<p>
Hmm, i couldn't figure out how to add the code to the patch. I tried 'sage -hg add amgf.py' as you suggested, Se'bastien, but that didn't work. So i finally copied and pasted it in. I hope that works. Let me know the proper way, yo.
</p>
<p>
Sorry for the delay.
</p>
<p>
Alex
</p>
TicketzafFri, 17 Jun 2011 20:47:36 GMTcc changed
https://trac.sagemath.org/ticket/10519#comment:20
https://trac.sagemath.org/ticket/10519#comment:20
<ul>
<li><strong>cc</strong>
<em>zaf</em> added
</li>
</ul>
TickettmonteilFri, 14 Oct 2011 16:38:52 GMTcc changed
https://trac.sagemath.org/ticket/10519#comment:21
https://trac.sagemath.org/ticket/10519#comment:21
<ul>
<li><strong>cc</strong>
<em>tmonteil</em> added
</li>
</ul>
TicketdavidloefflerSat, 10 Mar 2012 17:05:26 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519-fixed.patch</em>
</li>
</ul>
<p>
Patch against 5.0
</p>
TicketdavidloefflerSat, 10 Mar 2012 17:09:46 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/10519#comment:22
https://trac.sagemath.org/ticket/10519#comment:22
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>David Loeffler</em>
</li>
</ul>
<p>
Apply trac_10519-fixed.patch
</p>
<p>
(for the patchbot).
</p>
<p>
Alex's last "patch" wasn't a patch at all, just a Mercurial header with some Python code arbitrarily pasted into it. I've just uploaded a non-broken version of the patch. I also added the necessary import statements -- a file that's supposed to be part of the Sage library can't import the Sage library (with <code>from all_cmdline import *</code>), since that would lead to an infinite loop, so the necessary imports have to be done one-by-one.
</p>
<p>
The patch now passes doctests on my machine, but there are some worrying errors that came up when I ran a syntax checker on the new file:
{{{amgf.py:908: local variable 'R' is assigned to but never used
amgf.py:909: local variable 'd' is assigned to but never used
amgf.py:1070: local variable 'Ht' is assigned to but never used
amgf.py:1251: local variable 'Ht' is assigned to but never used
amgf.py:1591: undefined name 'n'
amgf.py:2199: local variable 'H' is assigned to but never used
amgf.py:2242: undefined name 'verdict'
}}}
The "undefined name" errors tend to suggest that some cases have bugs in them, *and* those cases are not checked in any doctest, which is a double whammy. Hence "needs work".
</p>
TicketdavidloefflerSat, 10 Mar 2012 17:10:34 GMT
https://trac.sagemath.org/ticket/10519#comment:23
https://trac.sagemath.org/ticket/10519#comment:23
<p>
Sorry, that list got mangled. It should be
</p>
<pre class="wiki">amgf.py:908: local variable 'R' is assigned to but never used
amgf.py:909: local variable 'd' is assigned to but never used
amgf.py:1070: local variable 'Ht' is assigned to but never used
amgf.py:1251: local variable 'Ht' is assigned to but never used
amgf.py:1591: undefined name 'n'
amgf.py:2199: local variable 'H' is assigned to but never used
amgf.py:2242: undefined name 'verdict'
</pre>
TicketaraichevSat, 10 Mar 2012 20:53:21 GMT
https://trac.sagemath.org/ticket/10519#comment:24
https://trac.sagemath.org/ticket/10519#comment:24
<p>
Thanks for your review, David. This patch was my first on Sage Trac and i've since learned a lot, thanks to the constructive feedback of fellow Sagers such as yourself.
</p>
<p>
Since amgf.py was somewhat of a code bomb, i decided to split it up into simpler pieces, submit those pieces, and, once they get accepted, recombine them. If you or anyone else would like to help me in this endeavor, check out the latest amgf piece at <a class="ext-link" href="http://trac.sagemath.org/sage_trac/ticket/12417"><span class="icon"></span>http://trac.sagemath.org/sage_trac/ticket/12417</a>.
</p>
<p>
Thanks!
</p>
<p>
I'll keep you all updated on the progress of amgf with these comments.
</p>
TicketaraichevThu, 09 Aug 2012 00:16:50 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519.2.patch</em>
</li>
</ul>
<p>
Major rewrite.
</p>
TicketaraichevThu, 09 Aug 2012 00:17:50 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:25
https://trac.sagemath.org/ticket/10519#comment:25
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketchapotonMon, 03 Sep 2012 18:37:55 GMT
https://trac.sagemath.org/ticket/10519#comment:26
https://trac.sagemath.org/ticket/10519#comment:26
<p>
Apply only trac_10519.2.patch
</p>
TicketchapotonMon, 03 Sep 2012 18:41:56 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/10519#comment:27
https://trac.sagemath.org/ticket/10519#comment:27
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>coverage, trailing whitespace</em>
</li>
</ul>
<ul><li>Please ensure that every function is doctested
</li></ul><ul><li>Please remove all trailing whitespaces
</li></ul>
TicketchapotonSat, 22 Sep 2012 18:22:02 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519-asymptotic_multiple-v3.patch</em>
</li>
</ul>
TicketchapotonSat, 22 Sep 2012 18:25:10 GMTdescription changed
https://trac.sagemath.org/ticket/10519#comment:28
https://trac.sagemath.org/ticket/10519#comment:28
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=28">diff</a>)
</li>
</ul>
<p>
I have removed all whitespaces in the new patch. All test pass.
</p>
<p>
<strong>BUT</strong> there remains 6 procedures with no doctest. Remember that it is neccesary to have 100% doctesting.
</p>
<p>
Also the name <strong>amgf</strong> is not good. It should be changed to something more descriptive.
</p>
TicketchapotonSat, 22 Sep 2012 18:25:57 GMTwork_issues changed
https://trac.sagemath.org/ticket/10519#comment:29
https://trac.sagemath.org/ticket/10519#comment:29
<ul>
<li><strong>work_issues</strong>
changed from <em>coverage, trailing whitespace</em> to <em>coverage</em>
</li>
</ul>
TicketaraichevMon, 24 Sep 2012 21:33:17 GMT
https://trac.sagemath.org/ticket/10519#comment:30
https://trac.sagemath.org/ticket/10519#comment:30
<p>
Thanks, folks. I'll document those outstanding 6 procedures within a week.
</p>
TicketaraichevMon, 01 Oct 2012 22:41:33 GMT
https://trac.sagemath.org/ticket/10519#comment:31
https://trac.sagemath.org/ticket/10519#comment:31
<p>
Hmm, i might take longer, because now that i've upgraded to Sage 5.3, i can't create a Sage clone :-( See <a class="ext-link" href="http://ask.sagemath.org/question/1821/cant-create-sage-clone-again"><span class="icon"></span>http://ask.sagemath.org/question/1821/cant-create-sage-clone-again</a>.
</p>
TicketaraichevTue, 02 Oct 2012 23:07:59 GMT
https://trac.sagemath.org/ticket/10519#comment:32
https://trac.sagemath.org/ticket/10519#comment:32
<p>
Still can't create a Sage clone. In the meantime, here's my patch as a Python file (which i tested on Sage 5.3 with sage -t --long) for anyone who wants to review it: <a class="ext-link" href="https://www.dropbox.com/s/fcxsbuw119qclxt/trac_10519v4.py"><span class="icon"></span>https://www.dropbox.com/s/fcxsbuw119qclxt/trac_10519v4.py</a>.
</p>
TicketaraichevSun, 07 Oct 2012 22:52:19 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519v4.patch</em>
</li>
</ul>
TicketaraichevSun, 07 Oct 2012 22:52:48 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:33
https://trac.sagemath.org/ticket/10519#comment:33
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketchapotonFri, 26 Oct 2012 13:32:04 GMTdescription changed
https://trac.sagemath.org/ticket/10519#comment:34
https://trac.sagemath.org/ticket/10519#comment:34
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=34">diff</a>)
</li>
</ul>
<p>
apply only trac_10519v4.patch
</p>
TicketchapotonFri, 26 Oct 2012 13:59:57 GMTstatus, work_issues changed
https://trac.sagemath.org/ticket/10519#comment:35
https://trac.sagemath.org/ticket/10519#comment:35
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
changed from <em>coverage</em> to <em>failing tests, trailing whitespaces</em>
</li>
</ul>
<p>
please remove all trailing whitespaces
</p>
<p>
please correct the failing doctests
</p>
TicketchapotonTue, 30 Oct 2012 20:03:12 GMTstatus, description changed
https://trac.sagemath.org/ticket/10519#comment:36
https://trac.sagemath.org/ticket/10519#comment:36
<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/10519?action=diff&version=36">diff</a>)
</li>
</ul>
<p>
apply trac_10519-v5.patch
</p>
TicketchapotonTue, 30 Oct 2012 20:48:44 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519-v5.patch</em>
</li>
</ul>
TicketchapotonTue, 30 Oct 2012 20:49:04 GMT
https://trac.sagemath.org/ticket/10519#comment:37
https://trac.sagemath.org/ticket/10519#comment:37
<p>
apply trac_10519-v5.patch
</p>
TicketaraichevSat, 03 Nov 2012 00:33:40 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519-v6.patch</em>
</li>
</ul>
TicketaraichevSat, 03 Nov 2012 00:35:42 GMT
https://trac.sagemath.org/ticket/10519#comment:38
https://trac.sagemath.org/ticket/10519#comment:38
<p>
Hmm, i don't understand why i didn't get any test errors when i submitted v4. Anyway, fixed the errors.
</p>
TicketaraichevSat, 03 Nov 2012 00:36:07 GMTwork_issues deleted
https://trac.sagemath.org/ticket/10519#comment:39
https://trac.sagemath.org/ticket/10519#comment:39
<ul>
<li><strong>work_issues</strong>
<em>failing tests, trailing whitespaces</em> deleted
</li>
</ul>
TicketchapotonSat, 03 Nov 2012 08:15:41 GMTdescription changed
https://trac.sagemath.org/ticket/10519#comment:40
https://trac.sagemath.org/ticket/10519#comment:40
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=40">diff</a>)
</li>
</ul>
<p>
Apply trac_10519-v6.patch
</p>
TicketchapotonSat, 03 Nov 2012 13:11:51 GMT
https://trac.sagemath.org/ticket/10519#comment:41
https://trac.sagemath.org/ticket/10519#comment:41
<p>
well, now version V5 works and version V6 does not pass the tests.
</p>
<p>
I had already made all the necessary corrections in version V5.
</p>
<p>
Can we just forget version V6 ?
</p>
TicketaraichevSun, 04 Nov 2012 20:38:24 GMT
https://trac.sagemath.org/ticket/10519#comment:42
https://trac.sagemath.org/ticket/10519#comment:42
<p>
OK, let's forget V6. I bet the problem is one or two docstring lines that output a dictionary. The random order of the keys is causing a test failure when it doesn't match up with the docstring key order. We can fix that by sorting the keys before printing them.
</p>
TicketchapotonMon, 05 Nov 2012 10:33:36 GMTdescription changed
https://trac.sagemath.org/ticket/10519#comment:43
https://trac.sagemath.org/ticket/10519#comment:43
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=43">diff</a>)
</li>
</ul>
<p>
apply trac_10519-v5.patch
</p>
TicketdkrennWed, 28 Nov 2012 09:53:29 GMTcc changed
https://trac.sagemath.org/ticket/10519#comment:44
https://trac.sagemath.org/ticket/10519#comment:44
<ul>
<li><strong>cc</strong>
<em>dkrenn</em> added
</li>
</ul>
TicketaraichevMon, 11 Feb 2013 20:59:54 GMT
https://trac.sagemath.org/ticket/10519#comment:45
https://trac.sagemath.org/ticket/10519#comment:45
<p>
Hi folks, is this patch ready to be added to Sage? If not, what's the next step?
</p>
TicketchapotonWed, 13 Feb 2013 11:15:39 GMT
https://trac.sagemath.org/ticket/10519#comment:46
https://trac.sagemath.org/ticket/10519#comment:46
<p>
The next step would be that somebody interested does a real review of this patch. This means looking seriously at the code, using the functions, trying to find bugs, and so on. I am not candidate, sorry.
</p>
TicketchapotonFri, 01 Mar 2013 09:29:55 GMT
https://trac.sagemath.org/ticket/10519#comment:47
https://trac.sagemath.org/ticket/10519#comment:47
<p>
Here is the report of pyflakes : some things that need to be corrected
</p>
<pre class="wiki">sage/combinat/asymptotics_multivariate_generating_functions.py:147: 'UniqueFactorizationDomains' imported but unused
sage/combinat/asymptotics_multivariate_generating_functions.py:156: 'CartesianProduct' imported but unused
sage/combinat/asymptotics_multivariate_generating_functions.py:171: 'ZZ' imported but unused
sage/combinat/asymptotics_multivariate_generating_functions.py:172: redefinition of unused 'PolynomialRing' from line 148
sage/combinat/asymptotics_multivariate_generating_functions.py:175: 'SageObject' imported but unused
sage/combinat/asymptotics_multivariate_generating_functions.py:1719: local variable 'Ht' is assigned to but never used
sage/combinat/asymptotics_multivariate_generating_functions.py:2097: local variable 'Ht' is assigned to but never used
sage/combinat/asymptotics_multivariate_generating_functions.py:2992: local variable 'X' is assigned to but never used
sage/combinat/asymptotics_multivariate_generating_functions.py:3083: undefined name 'Subsets'
</pre>
TicketchapotonFri, 01 Mar 2013 10:36:05 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/10519#comment:48
https://trac.sagemath.org/ticket/10519#comment:48
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>pyflakes report</em>
</li>
</ul>
TicketaraichevMon, 18 Mar 2013 04:18:44 GMTattachment set
https://trac.sagemath.org/ticket/10519
https://trac.sagemath.org/ticket/10519
<ul>
<li><strong>attachment</strong>
set to <em>trac_10519-v7.patch</em>
</li>
</ul>
TicketaraichevMon, 18 Mar 2013 04:20:01 GMT
https://trac.sagemath.org/ticket/10519#comment:49
https://trac.sagemath.org/ticket/10519#comment:49
<p>
Thanks. Fixed.
</p>
TicketaraichevMon, 18 Mar 2013 04:20:29 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:50
https://trac.sagemath.org/ticket/10519#comment:50
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketchapotonMon, 18 Mar 2013 09:04:00 GMTdescription changed; work_issues deleted
https://trac.sagemath.org/ticket/10519#comment:51
https://trac.sagemath.org/ticket/10519#comment:51
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=51">diff</a>)
</li>
<li><strong>work_issues</strong>
<em>pyflakes report</em> deleted
</li>
</ul>
<p>
for the bot
</p>
<p>
Apply trac_10519-v7.patch
</p>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/10519#comment:52
https://trac.sagemath.org/ticket/10519#comment:52
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketdimpaseThu, 21 Nov 2013 11:19:21 GMT
https://trac.sagemath.org/ticket/10519#comment:53
https://trac.sagemath.org/ticket/10519#comment:53
<p>
It does not seems to be properly integrated into sage:
</p>
<pre class="wiki">sage: from sage.combinat.asymptotics_multivariate_generating_functions import *
---------------------------------------------------------------------------
ImportError Traceback (most recent call last)
<ipython-input-1-3a279f9ff0e0> in <module>()
----> 1 from sage.combinat.asymptotics_multivariate_generating_functions import *
ImportError: No module named asymptotics_multivariate_generating_functions
sage:
</pre><p>
and indeed, you can see that the patch should be for 'sage/combinat' directory, not for './'.
Unless this is meant to go to the "combinat queue" - and I have no clue how such reviews are meant to happen and
how the corresponding code "admin" should look like.
</p>
<p>
Question: how much of this is dependent upon the "combinat queue"?
</p>
TicketaraichevThu, 21 Nov 2013 20:14:15 GMT
https://trac.sagemath.org/ticket/10519#comment:54
https://trac.sagemath.org/ticket/10519#comment:54
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:53" title="Comment 53">dimpase</a>:
</p>
<p>
Yes, the patch should be for <code>sage/combinat</code>. Feel free to change that, if you'd like to review the patch now. Otherwise, i'll change it when i get some time in the next week.
</p>
<blockquote class="citation">
<p>
It does not seems to be properly integrated into sage:
</p>
<pre class="wiki">sage: from sage.combinat.asymptotics_multivariate_generating_functions import *
---------------------------------------------------------------------------
ImportError Traceback (most recent call last)
<ipython-input-1-3a279f9ff0e0> in <module>()
----> 1 from sage.combinat.asymptotics_multivariate_generating_functions import *
ImportError: No module named asymptotics_multivariate_generating_functions
sage:
</pre><p>
and indeed, you can see that the patch should be for 'sage/combinat' directory, not for './'.
Unless this is meant to go to the "combinat queue" - and I have no clue how such reviews are meant to happen and
how the corresponding code "admin" should look like.
</p>
<p>
Question: how much of this is dependent upon the "combinat queue"?
</p>
</blockquote>
TicketdimpaseThu, 21 Nov 2013 20:20:29 GMT
https://trac.sagemath.org/ticket/10519#comment:55
https://trac.sagemath.org/ticket/10519#comment:55
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:54" title="Comment 54">araichev</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:53" title="Comment 53">dimpase</a>:
</p>
<p>
Yes, the patch should be for <code>sage/combinat</code>. Feel free to change that, if you'd like to review the patch now. Otherwise, i'll change it when i get some time in the next week.
</p>
</blockquote>
<p>
One should use <code>lazy_import</code> in <code>sage/combinat/all.py</code> to import the stuff in this patch, rather than
forcing the user to do the explicit import
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Question: how much of this is dependent upon the "combinat queue"?
</p>
</blockquote>
</blockquote>
<p>
So, how much, if anything at all?
</p>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/10519#comment:56
https://trac.sagemath.org/ticket/10519#comment:56
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TickettscrimSat, 15 Feb 2014 05:14:06 GMTcommit, branch set
https://trac.sagemath.org/ticket/10519#comment:57
https://trac.sagemath.org/ticket/10519#comment:57
<ul>
<li><strong>commit</strong>
set to <em>60b93376cbb072a1aab99646806555830799468f</em>
</li>
<li><strong>branch</strong>
set to <em>public/combinat/analytic-10519</em>
</li>
</ul>
<p>
Here's a git branch version along with some extensive review changes to the documentation. I made some internal methods private to cleanup the namespace (i.e. what you get with tab completion). I also made stuff python 3 compatible. Unfortunately I also cannot really give a good review to the mathematics behind this.
</p>
<p>
@Dima, I think you've misunderstood what the combinat queue is. However it's moot because we are no longer using hg.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b01625d4f74ad7b187707254c9e404c97ae7be6d"><span class="icon"></span>b01625d</a></td><td><code>10519: Removed unused modules and variables</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=443ab74ab4c0b87eaeae1d790b2abc850fc92377"><span class="icon"></span>443ab74</a></td><td><code>Work on cleaning up documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7f9b89ed07cb5053bce29a48748382da525f6d5c"><span class="icon"></span>7f9b89e</a></td><td><code>Merge branch 'develop' into public/combinat/analytic-10519</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=40dacc93e20e043d864470693957977fc03f2b25"><span class="icon"></span>40dacc9</a></td><td><code>Review changes and added new file to documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=60b93376cbb072a1aab99646806555830799468f"><span class="icon"></span>60b9337</a></td><td><code>Tweaked print statements and list().</code>
</td></tr></table>
TicketaraichevSat, 15 Feb 2014 22:06:17 GMT
https://trac.sagemath.org/ticket/10519#comment:58
https://trac.sagemath.org/ticket/10519#comment:58
<p>
Hooray! Thanks heaps, @tscrim.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:57" title="Comment 57">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Here's a git branch version along with some extensive review changes to the documentation. I made some internal methods private to cleanup the namespace (i.e. what you get with tab completion). I also made stuff python 3 compatible. Unfortunately I also cannot really give a good review to the mathematics behind this.
</p>
<p>
@Dima, I think you've misunderstood what the combinat queue is. However it's moot because we are no longer using hg.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b01625d4f74ad7b187707254c9e404c97ae7be6d"><span class="icon"></span>b01625d</a></td><td><code>10519: Removed unused modules and variables</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=443ab74ab4c0b87eaeae1d790b2abc850fc92377"><span class="icon"></span>443ab74</a></td><td><code>Work on cleaning up documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7f9b89ed07cb5053bce29a48748382da525f6d5c"><span class="icon"></span>7f9b89e</a></td><td><code>Merge branch 'develop' into public/combinat/analytic-10519</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=40dacc93e20e043d864470693957977fc03f2b25"><span class="icon"></span>40dacc9</a></td><td><code>Review changes and added new file to documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=60b93376cbb072a1aab99646806555830799468f"><span class="icon"></span>60b9337</a></td><td><code>Tweaked print statements and list().</code>
</td></tr></table>
</blockquote>
TicketdimpaseSat, 15 Feb 2014 22:53:44 GMT
https://trac.sagemath.org/ticket/10519#comment:59
https://trac.sagemath.org/ticket/10519#comment:59
<p>
Why does one need these FFPD and FFPDSum classes? Are Sage's multivariate rational functions not good enough?
One should have an interface between these FFPD and FFPDSum and "normal" Sage's rational functions, anyway.
</p>
<p>
Also, I think that the part that computes decomposition(s) of rational functions must be factored out (and put on another ticket, and they should not be in combinat/, as this has noting to do with combinatorics). It has independent uses, which need not have anything to do with the stated purpose of this ticket.
</p>
TicketdimpaseSat, 15 Feb 2014 23:05:06 GMT
https://trac.sagemath.org/ticket/10519#comment:60
https://trac.sagemath.org/ticket/10519#comment:60
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:57" title="Comment 57">tscrim</a>:
</p>
<blockquote class="citation">
<p>
@Dima, I think you've misunderstood what the combinat queue is. However it's moot because we are no longer using hg.
</p>
</blockquote>
<p>
Travis, I think already in 2010 I knew what the combinat queue is ;-)
</p>
<p>
Somehow, when reading the code, something made me wonder if this code depends on some features from (what was formerly known as?) combinat queue. I'm happy to know that it's not the case.
</p>
TickettscrimSun, 16 Feb 2014 03:10:15 GMT
https://trac.sagemath.org/ticket/10519#comment:61
https://trac.sagemath.org/ticket/10519#comment:61
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:60" title="Comment 60">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:57" title="Comment 57">tscrim</a>:
</p>
<blockquote class="citation">
<p>
@Dima, I think you've misunderstood what the combinat queue is. However it's moot because we are no longer using hg.
</p>
</blockquote>
<p>
Travis, I think already in 2010 I knew what the combinat queue is ;-)
</p>
<p>
Somehow, when reading the code, something made me wonder if this code depends on some features from (what was formerly known as?) combinat queue. I'm happy to know that it's not the case.
</p>
</blockquote>
<p>
Ah sorry, it seemed to me from <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:53" title="Comment 53">comment:53</a> like there was a misunderstanding. I'm also happy there's no dependence on any patches that are in the (now unused) queue that we might have to untangle.
</p>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/10519#comment:62
https://trac.sagemath.org/ticket/10519#comment:62
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
TicketrwsFri, 09 May 2014 15:39:47 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/10519#comment:63
https://trac.sagemath.org/ticket/10519#comment:63
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>rebase</em>
</li>
</ul>
TicketgitThu, 19 Jun 2014 20:03:50 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:64
https://trac.sagemath.org/ticket/10519#comment:64
<ul>
<li><strong>commit</strong>
changed from <em>60b93376cbb072a1aab99646806555830799468f</em> to <em>7511d1454940e11462bf07452c745638fd48d95a</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="http://git.sagemath.org/sage.git/commit/?id=7511d1454940e11462bf07452c745638fd48d95a"><span class="icon"></span>7511d14</a></td><td><code>Merge branch 'public/combinat/analytic-10519' of ssh://trac.sagemath.org:22/sage into 10519</code>
</td></tr></table>
TicketchapotonThu, 19 Jun 2014 20:05:57 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:65
https://trac.sagemath.org/ticket/10519#comment:65
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketvdelecroixThu, 19 Jun 2014 20:50:44 GMT
https://trac.sagemath.org/ticket/10519#comment:66
https://trac.sagemath.org/ticket/10519#comment:66
<p>
Hi,
</p>
<p>
Just read the code and glance some remarks. The code looks very interesting but I have some advices/questions.
</p>
<ul><li>as a matter of uniformization class name are better expanded (<code>FractionWithFactoredPolynomialDenominator</code> instead of <code>FFPD</code>)
</li><li><code>cartesian_product_iterator</code> will be very soonly deprecated, so use <code>product</code> from the Python <code>itertools</code> module instead.
</li><li>it is generally bad practice to have such a big import list at the begining of the file (because of potential circular import). Moreover some of them can be completely avoided: <code>L = FractionField(PolynomialRing(K, indets))</code> can be <code>PolynomialRing(K, indets).fraction_field()</code>.
</li></ul><p>
More seriously:
</p>
<ul><li>I do not understand the specifications. It is said that the numerator must be an element p of a 0 or 1 variate factorial polynomial ring R but you have examples involving <code>cos(x)</code>. What is the underlying polynomial ring in that case?
</li><li>repeating <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:59" title="Comment 59">comment:59</a>, Why there is a need for the class <code>FFDP</code>? Why isn't it directly implemented as the level of fractions of polynomials? It would makes sense to cache the factorization of the denominator if it is required in several places, but I do not see the point of the <code>FFDP</code> class.
</li><li>Some of the method needs definition: <code>critical_cone</code>, <code>is_convenient_multiple_point</code>, ...
</li><li>Some of the method would better be hidden: <code>combine_like_terms</code>, ...
</li></ul><p>
Best
Vincent
</p>
TicketaraichevSat, 21 Jun 2014 22:44:55 GMT
https://trac.sagemath.org/ticket/10519#comment:67
https://trac.sagemath.org/ticket/10519#comment:67
<p>
Hi folks, because of my new work and responsibilities outside the realm of analytic combinatorics and Sage, i've needed to put this project on hold indefinitely. Please feel free to change the code as you like. I hope i've documented it well enough with comments and references so that others can carry it to fruition.
</p>
TicketvdelecroixSat, 21 Jun 2014 23:38:09 GMT
https://trac.sagemath.org/ticket/10519#comment:68
https://trac.sagemath.org/ticket/10519#comment:68
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:67" title="Comment 67">araichev</a>:
</p>
<blockquote class="citation">
<p>
Hi folks, because of my new work and responsibilities outside the realm of analytic combinatorics and Sage, i've needed to put this project on hold indefinitely. Please feel free to change the code as you like. I hope i've documented it well enough with comments and references so that others can carry it to fruition.
</p>
</blockquote>
<p>
Hey Alexei,
</p>
<p>
Good luck with your new work. I will read it carefully and see what I can do. It is definitely a good piece of work.
</p>
<p>
Vincent
</p>
TickettscrimSun, 22 Jun 2014 04:25:05 GMT
https://trac.sagemath.org/ticket/10519#comment:69
https://trac.sagemath.org/ticket/10519#comment:69
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:68" title="Comment 68">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:67" title="Comment 67">araichev</a>:
</p>
<blockquote class="citation">
<p>
Hi folks, because of my new work and responsibilities outside the realm of analytic combinatorics and Sage, i've needed to put this project on hold indefinitely. Please feel free to change the code as you like. I hope i've documented it well enough with comments and references so that others can carry it to fruition.
</p>
</blockquote>
<p>
Hey Alexei,
</p>
<p>
Good luck with your new work.
</p>
</blockquote>
<p>
Yes, good luck with your new work.
</p>
<blockquote class="citation">
<p>
I will read it carefully and see what I can do. It is definitely a good piece of work.
</p>
</blockquote>
<p>
Vincent (or Dima), will you be able to check the math for this ticket? If not, I think I could go through it math-wise.
</p>
<p>
On the code front:
</p>
<ul><li>Since there are only two classes, I agree with you that we should spell it out.
</li><li>Similar for changing to the built-in python tools.
</li><li>However I think the big import statement is okay since this is going to be a leaf (or at least on a short branch) of the import tree/poset/digraph.
</li><li>What methods do you think should be private?
</li><li>What can I do code-wise to help get this in?
</li></ul>
TicketdimpaseMon, 23 Jun 2014 07:15:30 GMT
https://trac.sagemath.org/ticket/10519#comment:70
https://trac.sagemath.org/ticket/10519#comment:70
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:69" title="Comment 69">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:68" title="Comment 68">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:67" title="Comment 67">araichev</a>:
</p>
<blockquote class="citation">
<p>
Hi folks, because of my new work and responsibilities outside the realm of analytic combinatorics and Sage, i've needed to put this project on hold indefinitely. Please feel free to change the code as you like. I hope i've documented it well enough with comments and references so that others can carry it to fruition.
</p>
</blockquote>
<p>
Hey Alexei,
</p>
<p>
Good luck with your new work.
</p>
</blockquote>
<p>
Yes, good luck with your new work.
</p>
<blockquote class="citation">
<p>
I will read it carefully and see what I can do. It is definitely a good piece of work.
</p>
</blockquote>
<p>
Vincent (or Dima), will you be able to check the math for this ticket? If not, I think I could go through it math-wise.
</p>
</blockquote>
<p>
please see my #<a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:59" title="Comment 59">comment:59</a>
</p>
<p>
IMHO refactoring and interfacing with Sage's facilities for rational functions is due, before this can be merged.
</p>
Ticketvbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/10519#comment:71
https://trac.sagemath.org/ticket/10519#comment:71
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketdkrennMon, 18 Aug 2014 19:46:11 GMTbranch changed
https://trac.sagemath.org/ticket/10519#comment:72
https://trac.sagemath.org/ticket/10519#comment:72
<ul>
<li><strong>branch</strong>
changed from <em>public/combinat/analytic-10519</em> to <em>public/combinat/analytic-10519-on-6.3</em>
</li>
</ul>
TicketdkrennMon, 18 Aug 2014 19:46:49 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:73
https://trac.sagemath.org/ticket/10519#comment:73
<ul>
<li><strong>commit</strong>
changed from <em>7511d1454940e11462bf07452c745638fd48d95a</em> to <em>28057320fe66a23041b55a7769e4607ca4d8abbd</em>
</li>
</ul>
<p>
rebased to 6.3
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=761ea52e2f1aa7663967ccf0580f7582a3327fe6"><span class="icon"></span>761ea52</a></td><td><code>10519: Removed unused modules and variables</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=6701fb440c7dfc4fb504fab0c75ce840994230bd"><span class="icon"></span>6701fb4</a></td><td><code>Work on cleaning up documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=46ad95836b08dfa8fe3664a961a7ec7068e89da2"><span class="icon"></span>46ad958</a></td><td><code>Review changes and added new file to documentation.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=28057320fe66a23041b55a7769e4607ca4d8abbd"><span class="icon"></span>2805732</a></td><td><code>Tweaked print statements and list().</code>
</td></tr></table>
TicketdkrennMon, 18 Aug 2014 19:47:58 GMTdependencies set
https://trac.sagemath.org/ticket/10519#comment:74
https://trac.sagemath.org/ticket/10519#comment:74
<ul>
<li><strong>dependencies</strong>
set to <em>#16848</em>
</li>
</ul>
<p>
There is a doctest (and the doctest using the result below) failing:
</p>
<pre class="wiki">sage -t src/sage/combinat/asymptotics_multivariate_generating_functions.py
**********************************************************************
File "src/sage/combinat/asymptotics_multivariate_generating_functions.py", line 90, in sage.combinat.asymptotics_multivariate_generating_functions
Failed example:
s
Expected:
[{x: 1, y: 1}]
Got:
[]
</pre><p>
This is now <a class="new ticket" href="https://trac.sagemath.org/ticket/16848" title="defect: solve of ideal over QQ[x,y] does not give solution anymore (since 6.3) (new)">#16848</a>.
</p>
TicketdkrennMon, 18 Aug 2014 19:48:16 GMTreviewer changed
https://trac.sagemath.org/ticket/10519#comment:75
https://trac.sagemath.org/ticket/10519#comment:75
<ul>
<li><strong>reviewer</strong>
changed from <em>David Loeffler</em> to <em>Daniel Krenn, David Loeffler</em>
</li>
</ul>
TicketdkrennMon, 18 Aug 2014 19:50:51 GMT
https://trac.sagemath.org/ticket/10519#comment:76
https://trac.sagemath.org/ticket/10519#comment:76
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:70" title="Comment 70">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:69" title="Comment 69">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Vincent (or Dima), will you be able to check the math for this ticket? If not, I think I could go through it math-wise.
</p>
</blockquote>
</blockquote>
<p>
At least, I started checking the math.
</p>
<blockquote class="citation">
<p>
IMHO refactoring and interfacing with Sage's facilities for rational functions is due, before this can be merged.
</p>
</blockquote>
<p>
Yes, this should be definitely done.
</p>
TicketdkrennMon, 18 Aug 2014 19:51:30 GMTwork_issues deleted
https://trac.sagemath.org/ticket/10519#comment:77
https://trac.sagemath.org/ticket/10519#comment:77
<ul>
<li><strong>work_issues</strong>
<em>rebase</em> deleted
</li>
</ul>
TicketgitTue, 19 Aug 2014 13:14:29 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:78
https://trac.sagemath.org/ticket/10519#comment:78
<ul>
<li><strong>commit</strong>
changed from <em>28057320fe66a23041b55a7769e4607ca4d8abbd</em> to <em>8bc997179b09a47eb0b23fc3c623ec8b92391e8f</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="http://git.sagemath.org/sage.git/commit/?id=8bc997179b09a47eb0b23fc3c623ec8b92391e8f"><span class="icon"></span>8bc9971</a></td><td><code>fixed broken doctest</code>
</td></tr></table>
TicketdkrennTue, 19 Aug 2014 13:15:37 GMT
https://trac.sagemath.org/ticket/10519#comment:79
https://trac.sagemath.org/ticket/10519#comment:79
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:74" title="Comment 74">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
There is a doctest (and the doctest using the result below) failing:
</p>
<pre class="wiki">sage -t src/sage/combinat/asymptotics_multivariate_generating_functions.py
**********************************************************************
File "src/sage/combinat/asymptotics_multivariate_generating_functions.py", line 90, in sage.combinat.asymptotics_multivariate_generating_functions
Failed example:
s
Expected:
[{x: 1, y: 1}]
Got:
[]
</pre><p>
This is now <a class="new ticket" href="https://trac.sagemath.org/ticket/16848" title="defect: solve of ideal over QQ[x,y] does not give solution anymore (since 6.3) (new)">#16848</a>.
</p>
</blockquote>
<p>
This works now (by a workaround).
</p>
TicketdkrennTue, 19 Aug 2014 13:27:57 GMTstatus changed; dependencies deleted
https://trac.sagemath.org/ticket/10519#comment:80
https://trac.sagemath.org/ticket/10519#comment:80
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
<em>#16848</em> deleted
</li>
</ul>
TicketgitSat, 27 Dec 2014 08:41:53 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:81
https://trac.sagemath.org/ticket/10519#comment:81
<ul>
<li><strong>commit</strong>
changed from <em>8bc997179b09a47eb0b23fc3c623ec8b92391e8f</em> to <em>4a71e9c6c447323c20b7b0904e62084abad15c98</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="http://git.sagemath.org/sage.git/commit/?id=4a71e9c6c447323c20b7b0904e62084abad15c98"><span class="icon"></span>4a71e9c</a></td><td><code>Merge branch 'public/combinat/analytic-10519-on-6.3' of ssh://trac.sagemath.org:22/sage into 6.5.b4</code>
</td></tr></table>
TicketdkrennSat, 27 Dec 2014 15:24:06 GMT
https://trac.sagemath.org/ticket/10519#comment:82
https://trac.sagemath.org/ticket/10519#comment:82
<p>
I'm currently porting the code to Sage's parent/element framework.
</p>
TicketchapotonSat, 27 Dec 2014 16:13:30 GMT
https://trac.sagemath.org/ticket/10519#comment:83
https://trac.sagemath.org/ticket/10519#comment:83
<p>
My pep8 compliant branch is now available at "u/chapoton/10519".
</p>
<p>
I am not sure it will be easy to merge it with your other changes. Maybe it is not worth the trouble.
</p>
TicketdkrennMon, 29 Dec 2014 09:12:01 GMTbranch changed
https://trac.sagemath.org/ticket/10519#comment:84
https://trac.sagemath.org/ticket/10519#comment:84
<ul>
<li><strong>branch</strong>
changed from <em>public/combinat/analytic-10519-on-6.3</em> to <em>public/combinat/10519</em>
</li>
</ul>
TicketdkrennMon, 29 Dec 2014 09:13:58 GMTcommit, author changed
https://trac.sagemath.org/ticket/10519#comment:85
https://trac.sagemath.org/ticket/10519#comment:85
<ul>
<li><strong>commit</strong>
changed from <em>4a71e9c6c447323c20b7b0904e62084abad15c98</em> to <em>a54855dfb4c9855d2facb682542c5058534a276c</em>
</li>
<li><strong>author</strong>
changed from <em>Alex Raichev</em> to <em>Daniel Krenn, Alex Raichev</em>
</li>
</ul>
<p>
Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=94d860d00013d6176b5bfa6bf72b468987310466"><span class="icon"></span>94d860d</a></td><td><code>implement equality testing (using coercion model)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=38b0f7e183f05dad96a326d51d83282bb57a1e7c"><span class="icon"></span>38b0f7e</a></td><td><code>coercion from other FFPD rings implemented</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c37addd930f41733556d8c608ba1805dd99a1fd4"><span class="icon"></span>c37addd</a></td><td><code>trac #10519 pep8 fully compliant</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=79e88d1a16f5cd24bbaff988362b1810d784974b"><span class="icon"></span>79e88d1</a></td><td><code>moved code like in "9914678 - moved static methods to parent" to allow merging</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=53d60ea14b049384d1f4becba615e37f05530f43"><span class="icon"></span>53d60ea</a></td><td><code>Merge branch 'u/chapoton/10519' into t/10519-on-6.5.beta4</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=6a326a2c24c48370516484afe8efe2a96f467db1"><span class="icon"></span>6a326a2</a></td><td><code>fixed doctests (ordering of dicts has changed)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=494cd165606b2e9e7c501e12700045feba27c401"><span class="icon"></span>494cd16</a></td><td><code>fixed doctests (x and y are ordered as y, x)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8dc631138a8a8d7c3bf4f98c364f77fdb35fdb02"><span class="icon"></span>8dc6311</a></td><td><code>clauses for "R is None" removed</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9654c94b5c1e339b8f697ec516c198c230b5e77d"><span class="icon"></span>9654c94</a></td><td><code>restructure import of libraries</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a54855dfb4c9855d2facb682542c5058534a276c"><span class="icon"></span>a54855d</a></td><td><code>update authors, copyright; new reference; some empty lines inserted</code>
</td></tr></table>
TicketdkrennMon, 29 Dec 2014 09:25:23 GMT
https://trac.sagemath.org/ticket/10519#comment:86
https://trac.sagemath.org/ticket/10519#comment:86
<p>
I've uploaded what I have up to now. There is one major change, namely transitioning to Sage's parent/element framework. There are a couple of minor issues to fix left and some doctests are still failing (this is due to using a wrong base ring...)
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:83" title="Comment 83">chapoton</a>:
</p>
<blockquote class="citation">
<p>
My pep8 compliant branch is now available at "u/chapoton/10519".
I am not sure it will be easy to merge it with your other changes. Maybe it is not worth the trouble.
</p>
</blockquote>
<p>
I've merged your changes/branch (most of them manually; I think I've got all but please check if in doubt).
</p>
<p>
In "u/chapoton/10519" Sage 6.5.beta4 was merged, which made some doctests failing. I've created a commit fixing most of them, but there is one annoying issue: The doctest in line 3672
</p>
<pre class="wiki">DD = FFDR._diff_op(A, B, AB_derivs, T, M, 1, 2)
</pre><p>
takes now forever (over 45 seconds compared to 1.78 seconds before). Does someone has an idea what between 6.3 and 6.5.beta4 could trigger this?
</p>
TicketgitMon, 29 Dec 2014 12:32:32 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:87
https://trac.sagemath.org/ticket/10519#comment:87
<ul>
<li><strong>commit</strong>
changed from <em>a54855dfb4c9855d2facb682542c5058534a276c</em> to <em>301843b542aa2a6d326e394853ea3b1de0af2e6e</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="http://git.sagemath.org/sage.git/commit/?id=301843b542aa2a6d326e394853ea3b1de0af2e6e"><span class="icon"></span>301843b</a></td><td><code>fixed long doctest (introduced in c37addd - trac #10519 pep8 fully compliant)</code>
</td></tr></table>
TicketdkrennMon, 29 Dec 2014 12:36:37 GMT
https://trac.sagemath.org/ticket/10519#comment:88
https://trac.sagemath.org/ticket/10519#comment:88
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:86" title="Comment 86">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:83" title="Comment 83">chapoton</a>:
</p>
<blockquote class="citation">
<p>
My pep8 compliant branch is now available at "u/chapoton/10519".
</p>
</blockquote>
<p>
In "u/chapoton/10519" [...] The doctest in line 3672
</p>
<pre class="wiki">DD = FFDR._diff_op(A, B, AB_derivs, T, M, 1, 2)
</pre><p>
takes now forever (over 45 seconds compared to 1.78 seconds before). Does someone has an idea what between 6.3 and 6.5.beta4 could trigger this?
</p>
</blockquote>
<p>
Finally found the problem; wasn't the Sage upgrade, but commit c37addd "trac <a class="closed ticket" href="https://trac.sagemath.org/ticket/10519" title="enhancement: analytic combinatorics: new code for computing asymptotics for ... (closed: fixed)">#10519</a> pep8 fully compliant", which simplified
</p>
<pre class="wiki">if something != Integer(0):
</pre><p>
to
</p>
<pre class="wiki">if something:
</pre><p>
which made this test slow (don't know why...). Anyhow, pushed a commit which fixes this.
</p>
TicketchapotonMon, 29 Dec 2014 13:17:39 GMT
https://trac.sagemath.org/ticket/10519#comment:89
https://trac.sagemath.org/ticket/10519#comment:89
<p>
Oops, sorry for this. I thought it was simpler. I do not understand why is is much slower. Anyway, sorry again for the trouble.
</p>
TicketgitMon, 29 Dec 2014 19:36:20 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:90
https://trac.sagemath.org/ticket/10519#comment:90
<ul>
<li><strong>commit</strong>
changed from <em>301843b542aa2a6d326e394853ea3b1de0af2e6e</em> to <em>898a1c63ed95aa3c1250727c1c4c9e637f71f26a</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="http://git.sagemath.org/sage.git/commit/?id=61946d3f88e06f9af3a065cc17deb57bc2dcfb83"><span class="icon"></span>61946d3</a></td><td><code>rename reduce_ to reduce</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=84d797c2be1f547712a0e507111dae08debf7a28"><span class="icon"></span>84d797c</a></td><td><code>fixed univariate_decomposition when numerator is not a polynomial</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=898a1c63ed95aa3c1250727c1c4c9e637f71f26a"><span class="icon"></span>898a1c6</a></td><td><code>fix remaining failing doctests and code cleanup</code>
</td></tr></table>
TicketgitThu, 01 Jan 2015 18:39:48 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:91
https://trac.sagemath.org/ticket/10519#comment:91
<ul>
<li><strong>commit</strong>
changed from <em>898a1c63ed95aa3c1250727c1c4c9e637f71f26a</em> to <em>b0b5c3f303c913687a26f6cacd9c1691b0135f96</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="http://git.sagemath.org/sage.git/commit/?id=5e77b11034d45757e2bbb9e2fccd81d5b61c6297"><span class="icon"></span>5e77b11</a></td><td><code>short code simplification (FractionField) (comment:66 of #10519)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=438bd665e0daf7c493b37e14c38ad66f28ca9239"><span class="icon"></span>438bd66</a></td><td><code>make combine_like_terms private (comment:66 of #10519)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c2b0dab073b6f0aac3ae4d72a44b41ce85cee4e7"><span class="icon"></span>c2b0dab</a></td><td><code>allow coercion from fraction field of polynomial rings</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=41e995696bac0abb5cffc9be515a7e44ac886b07"><span class="icon"></span>41e9956</a></td><td><code>rename FFPDElement and FFPDSum</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b0b5c3f303c913687a26f6cacd9c1691b0135f96"><span class="icon"></span>b0b5c3f</a></td><td><code>repr of FractionWithFactoredDenominatorSum returns + between terms</code>
</td></tr></table>
TicketgitSun, 04 Jan 2015 14:17:53 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:92
https://trac.sagemath.org/ticket/10519#comment:92
<ul>
<li><strong>commit</strong>
changed from <em>b0b5c3f303c913687a26f6cacd9c1691b0135f96</em> to <em>974d997af25eb9239d016da2667e38d47b7baa8c</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="http://git.sagemath.org/sage.git/commit/?id=a7da4034a5d212ac317aa5d1c56f0317a63ed35d"><span class="icon"></span>a7da403</a></td><td><code>remove .list, since not needed any may lead to confusion</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a3bc051976b51877b8c277ca61d9989dee33e8a6"><span class="icon"></span>a3bc051</a></td><td><code>some small changes in code during review</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=edd992fd89ee93e4cd62d29ec1b3f24d17703ee6"><span class="icon"></span>edd992f</a></td><td><code>add doc (so that it is built automatically)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=605e62d76a406ed4c6bee2794ad5bd0e3fde10ca"><span class="icon"></span>605e62d</a></td><td><code>changes in docstrings during review</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=974d997af25eb9239d016da2667e38d47b7baa8c"><span class="icon"></span>974d997</a></td><td><code>make coerce_point private</code>
</td></tr></table>
TicketdkrennSun, 04 Jan 2015 14:29:49 GMT
https://trac.sagemath.org/ticket/10519#comment:93
https://trac.sagemath.org/ticket/10519#comment:93
<p>
During review I've worked over the code and ported it to Sage's parent/element framework.
</p>
<p>
Here are comments to the comments above:
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:66" title="Comment 66">vdelecroix</a>:
</p>
<blockquote class="citation">
<ul><li>as a matter of uniformization class name are better expanded (<code>FractionWithFactoredPolynomialDenominator</code> instead of <code>FFPD</code>)
</li></ul></blockquote>
<p>
Renaming done.
</p>
<blockquote class="citation">
<ul><li><code>cartesian_product_iterator</code> will be very soonly deprecated, so use <code>product</code> from the Python <code>itertools</code> module instead.
</li></ul></blockquote>
<p>
<code>itertools</code> are now used here.
</p>
<blockquote class="citation">
<ul><li>it is generally bad practice to have such a big import list at the begining of the file (because of potential circular import). Moreover some of them can be completely avoided: <code>L = FractionField(PolynomialRing(K, indets))</code> can be <code>PolynomialRing(K, indets).fraction_field()</code>.
</li></ul></blockquote>
<p>
Moved imports to functions. Only a very few global (in the module) imports are left.
</p>
<blockquote class="citation">
<p>
More seriously:
</p>
<ul><li>I do not understand the specifications. It is said that the numerator must be an element p of a 0 or 1 variate factorial polynomial ring R but you have examples involving <code>cos(x)</code>. What is the underlying polynomial ring in that case?
</li><li>repeating <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:59" title="Comment 59">comment:59</a>, Why there is a need for the class <code>FFDP</code>? Why isn't it directly implemented as the level of fractions of polynomials? It would makes sense to cache the factorization of the denominator if it is required in several places, but I do not see the point of the <code>FFDP</code> class.
</li></ul></blockquote>
<p>
The numerator can be any element from a ring into which the denominator coerces in. E.g. the denominator is a polynomial ring, but numerator is allowed to be out of the symbolic ring.
</p>
<blockquote class="citation">
<ul><li>Some of the method needs definition: <code>critical_cone</code>, <code>is_convenient_multiple_point</code>, ...
</li></ul></blockquote>
<p>
Tried a bit of rephrasing, but they are defined in the cited works.
</p>
<blockquote class="citation">
<ul><li>Some of the method would better be hidden: <code>combine_like_terms</code>, ...
</li></ul></blockquote>
<p>
Done. (also some other functions/methods were made private).
</p>
<p>
Concerning the review:
</p>
<ul><li>I've did a basic check on the mathematics: seems to be ok.
</li><li>I've read the docstrings and modified some of them either to make them clearer or to fit the guidelines in the developer guide (e.g. INPUT/OUTPUT blocks and so on). This is fine for me here (but documentation can, of course, always be improved ;) ).
</li><li>Documentation builds.
</li><li>All doctests pass.
</li><li>I'm towards a positive review, but, of course, someone has to cross review my transition of the code to parent/element framework.
</li></ul><p>
<strong>Therefore: needs cross-review '''
</strong></p>
TicketdkrennSun, 04 Jan 2015 14:30:13 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:94
https://trac.sagemath.org/ticket/10519#comment:94
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketmmezzarobbaFri, 23 Jan 2015 14:12:46 GMT
https://trac.sagemath.org/ticket/10519#comment:95
https://trac.sagemath.org/ticket/10519#comment:95
<p>
Just two minor comments:
</p>
<ul><li>If <code>FractionWithFactoredDenominatorRing</code> is intended to actually be a ring, shouldn't the numerators also be restricted to some given parent? (i.e., shouldn't there be a <code>numerator_ring</code> and a <code>denominator_ring</code>, or something like that, instead of just a <code>ring</code>?)
</li><li>Isn't at least some of the code general-purpose enough that it could live in <code>rings/</code> or <code>symbolic/</code> rather than <code>combinat/</code>? If not, perhaps it would be worth at least linking to it from the documentation of related objects.
</li></ul>
TicketdimpaseFri, 23 Jan 2015 14:15:30 GMT
https://trac.sagemath.org/ticket/10519#comment:96
https://trac.sagemath.org/ticket/10519#comment:96
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:95" title="Comment 95">mmezzarobba</a>:
</p>
<blockquote class="citation">
<ul><li>Isn't at least some of the code general-purpose enough that it could live in <code>rings/</code> or <code>symbolic/</code> rather than <code>combinat/</code>? If not, perhaps it would be worth at least linking to it from the documentation of related objects.
</li></ul></blockquote>
<p>
I have mentioned that in an old comment; IMHO the code must be refactored: the code to decompose a rational function into a sum must not live in combinat/.
</p>
TicketmmezzarobbaFri, 23 Jan 2015 14:27:05 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:97
https://trac.sagemath.org/ticket/10519#comment:97
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketrwsFri, 23 Jan 2015 16:17:00 GMT
https://trac.sagemath.org/ticket/10519#comment:98
https://trac.sagemath.org/ticket/10519#comment:98
<p>
Moreover, I could reproduce the results from the four doctests of <code>univariate_decomposition</code> using the existing <code>QuotientFields.element_class.partial_fraction_decomposition</code> and <code>Expression.partial_fraction</code> functionality. So, why not use these?
</p>
TicketgitTue, 28 Jul 2015 18:28:46 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:99
https://trac.sagemath.org/ticket/10519#comment:99
<ul>
<li><strong>commit</strong>
changed from <em>974d997af25eb9239d016da2667e38d47b7baa8c</em> to <em>1c995c5f965e6218f4e1f47082106d616be027ac</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="http://git.sagemath.org/sage.git/commit/?id=137f3c7eec8987a4c58c62a82d3e8629b29c523e"><span class="icon"></span>137f3c7</a></td><td><code>Worked in bugs reported by Mark Wilson</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1a7d977bdff9005e0b44abc60d9160c52bde0ed3"><span class="icon"></span>1a7d977</a></td><td><code>Merge tag '6.6' into t/10519-on-6.6</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1c995c5f965e6218f4e1f47082106d616be027ac"><span class="icon"></span>1c995c5</a></td><td><code>Merge tag '6.8' into t/10519-on-6.8</code>
</td></tr></table>
TicketdkrennTue, 28 Jul 2015 18:29:22 GMT
https://trac.sagemath.org/ticket/10519#comment:100
https://trac.sagemath.org/ticket/10519#comment:100
<p>
Merged in current stable <code>SageMath 6.8</code>
</p>
TicketgitSun, 31 Jan 2016 08:09:37 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:101
https://trac.sagemath.org/ticket/10519#comment:101
<ul>
<li><strong>commit</strong>
changed from <em>1c995c5f965e6218f4e1f47082106d616be027ac</em> to <em>a62ed348cc2fe2c9eafee35d7fcab42ee90f7000</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="http://git.sagemath.org/sage.git/commit/?id=0afbbdfaf9112665e61688751a00b58f45671cc1"><span class="icon"></span>0afbbdf</a></td><td><code>Merge tag '7.0' into t/10519/public/combinat/10519</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d6f58ce4468485f6ed28b2433085c51790c6d8cf"><span class="icon"></span>d6f58ce</a></td><td><code>Trac #10519: fix doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=564d8bd1a737594dc320a84cf0db218398955010"><span class="icon"></span>564d8bd</a></td><td><code>Trac #10519: move to sage.rings.asymptotic</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a62ed348cc2fe2c9eafee35d7fcab42ee90f7000"><span class="icon"></span>a62ed34</a></td><td><code>Trac #10519: fix doctests (new location)</code>
</td></tr></table>
TicketdkrennSun, 31 Jan 2016 08:10:48 GMT
https://trac.sagemath.org/ticket/10519#comment:102
https://trac.sagemath.org/ticket/10519#comment:102
<p>
Merged in current stable <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a> 7.0
</p>
TicketdkrennSun, 31 Jan 2016 08:12:57 GMT
https://trac.sagemath.org/ticket/10519#comment:103
https://trac.sagemath.org/ticket/10519#comment:103
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:95" title="Comment 95">mmezzarobba</a>:
</p>
<blockquote class="citation">
<ul><li>Isn't at least some of the code general-purpose enough that it could live in <code>rings/</code> or <code>symbolic/</code> rather than <code>combinat/</code>? If not, perhaps it would be worth at least linking to it from the documentation of related objects.
</li></ul></blockquote>
<p>
Moved to <code>sage.rings.asymptotic</code> (which was created last summer for asymptotic expansions <a class="new ticket" href="https://trac.sagemath.org/ticket/17601" title="enhancement: Meta ticket: Asymptotic Expansions in SageMath (new)">#17601</a>) as this seems very suitable.
</p>
TicketgitSun, 31 Jan 2016 08:17:55 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:104
https://trac.sagemath.org/ticket/10519#comment:104
<ul>
<li><strong>commit</strong>
changed from <em>a62ed348cc2fe2c9eafee35d7fcab42ee90f7000</em> to <em>6a44c8c436f7227a0bf1bb0dccc0ba94886d5801</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="http://git.sagemath.org/sage.git/commit/?id=6a44c8c436f7227a0bf1bb0dccc0ba94886d5801"><span class="icon"></span>6a44c8c</a></td><td><code>Trac #10519: update index in doc tree</code>
</td></tr></table>
TicketgitSun, 31 Jan 2016 12:50:55 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:105
https://trac.sagemath.org/ticket/10519#comment:105
<ul>
<li><strong>commit</strong>
changed from <em>6a44c8c436f7227a0bf1bb0dccc0ba94886d5801</em> to <em>1f21184f37244dbdf0395c75f474e4ca9429557f</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="http://git.sagemath.org/sage.git/commit/?id=b99c64a02508f2962c5eb34b7a5a1f65b37d09d5"><span class="icon"></span>b99c64a</a></td><td><code>Trac #10519: introduce numerator_ring and denominator_ring</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=328a75dc88c66bfa1e1f90543717b02ff14fec12"><span class="icon"></span>328a75d</a></td><td><code>Trac #10519: fix doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=34845ffb2e92ab2e9060566d665a6809b73f1283"><span class="icon"></span>34845ff</a></td><td><code>Trac #10519: speed up for univariate_decomposition</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9699c672b33748b3615e6bd5f421f8358af1451a"><span class="icon"></span>9699c67</a></td><td><code>Trac #10519: fix ReST documentation</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=340c34d8c817762f2eff9d4c6fecea089408eedb"><span class="icon"></span>340c34d</a></td><td><code>Trac #10519: mark class as experimental</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=874b4c33fb80c94159adba46f8de3a6a4e828460"><span class="icon"></span>874b4c3</a></td><td><code>Trac #10519: update authors and copyright</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1f21184f37244dbdf0395c75f474e4ca9429557f"><span class="icon"></span>1f21184</a></td><td><code>Trac #10519: fix indention in ReST</code>
</td></tr></table>
TicketdkrennSun, 31 Jan 2016 12:53:18 GMT
https://trac.sagemath.org/ticket/10519#comment:106
https://trac.sagemath.org/ticket/10519#comment:106
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:96" title="Comment 96">dimpase</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:95" title="Comment 95">mmezzarobba</a>:
</p>
<blockquote class="citation">
<ul><li>Isn't at least some of the code general-purpose enough that it could live in <code>rings/</code> or <code>symbolic/</code> rather than <code>combinat/</code>? If not, perhaps it would be worth at least linking to it from the documentation of related objects.
</li></ul></blockquote>
<p>
I have mentioned that in an old comment; IMHO the code must be refactored: the code to decompose a rational function into a sum must not live in combinat/.
</p>
</blockquote>
<p>
Changed; now <code>sage.rings.asymptotic</code>, see <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:103" title="Comment 103">comment:103</a>.
</p>
TicketdkrennSun, 31 Jan 2016 12:54:18 GMT
https://trac.sagemath.org/ticket/10519#comment:107
https://trac.sagemath.org/ticket/10519#comment:107
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:95" title="Comment 95">mmezzarobba</a>:
</p>
<blockquote class="citation">
<p>
Just two minor comments:
</p>
<ul><li>If <code>FractionWithFactoredDenominatorRing</code> is intended to actually be a ring, shouldn't the numerators also be restricted to some given parent? (i.e., shouldn't there be a <code>numerator_ring</code> and a <code>denominator_ring</code>, or something like that, instead of just a <code>ring</code>?)
</li></ul></blockquote>
<p>
I agree; now there is.
</p>
TicketdkrennSun, 31 Jan 2016 12:59:09 GMT
https://trac.sagemath.org/ticket/10519#comment:108
https://trac.sagemath.org/ticket/10519#comment:108
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:98" title="Comment 98">rws</a>:
</p>
<blockquote class="citation">
<p>
Moreover, I could reproduce the results from the four doctests of <code>univariate_decomposition</code> using the existing <code>QuotientFields.element_class.partial_fraction_decomposition</code> and <code>Expression.partial_fraction</code> functionality. So, why not use these?
</p>
</blockquote>
<p>
In <code>univariate_decomposition</code> the already-known factorization of the denominator is used. This gives a speed-up for not too small instances. Below the timings,
</p>
<ul><li>No speedup for the first (small) example in the doctest:
<pre class="wiki">sage: sage: from sage.rings.asymptotic.asymptotics_multivariate_generating_functions import FractionWithFactoredDenominatorRing
sage: sage: R.<x> = PolynomialRing(QQ)
sage: sage: FFPD = FractionWithFactoredDenominatorRing(R)
/local/dakrenn/sage/7.0/local/lib/python2.7/site-packages/sage/structure/unique_representation.py:1021: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
See http://trac.sagemath.org/10519 for details.
instance = typecall(cls, *args, **options)
sage: sage: f = 5*x^3 + 1/x + 1/(x-1) + 1/(3*x^2 + 1)
sage: sage: ff = FFPD(f)
sage: sage: %timeit -n 1 -r 1 ff.univariate_decomposition()
1 loops, best of 1: 2.24 ms per loop
sage: sage: %timeit -n 1 -r 1 f.partial_fraction_decomposition()
1 loops, best of 1: 1.02 ms per loop
</pre></li><li>But adding some more terms, we have a speedup:
<pre class="wiki">sage: sage: from sage.rings.asymptotic.asymptotics_multivariate_generating_functions import FractionWithFactoredDenominatorRing
sage: sage: R.<x> = PolynomialRing(QQ)
sage: sage: FFPD = FractionWithFactoredDenominatorRing(R)
/local/dakrenn/sage/7.0/local/lib/python2.7/site-packages/sage/structure/unique_representation.py:1021: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
See http://trac.sagemath.org/10519 for details.
instance = typecall(cls, *args, **options)
sage: sage: f = 5*x^3 + 1/x + 1/(x-1) + 1/(3*x^2 + 1) + 1/(x-2) + 1/(x-3) + 1/(x-4) + 1/(x-5) + 1/(x-6) + 1/(x-7) # + 1/(x-8) + 1/(x-9) + 1/(x-10) + 1/(x-11) + 1/(x-12) + 1/(x-13)
sage: sage: ff = FFPD(f)
sage: sage: %timeit -n 1 -r 1 ff.univariate_decomposition()
1 loops, best of 1: 1.53 ms per loop
sage: sage: %timeit -n 1 -r 1 f.partial_fraction_decomposition()
1 loops, best of 1: 3.3 ms per loop
</pre></li><li>The second example of the doctest is already faster with <code>univariate_decomposition</code> as it is:
<pre class="wiki">sage: sage: from sage.rings.asymptotic.asymptotics_multivariate_generating_functions import FractionWithFactoredDenominatorRing
sage: sage: R.<x> = PolynomialRing(QQ)
sage: sage: FFPD = FractionWithFactoredDenominatorRing(R, SR)
/local/dakrenn/sage/7.0/local/lib/python2.7/site-packages/sage/structure/unique_representation.py:1021: FutureWarning: This class/method/function is marked as experimental. It, its functionality or its interface might change without a formal deprecation.
See http://trac.sagemath.org/10519 for details.
instance = typecall(cls, *args, **options)
sage: sage: f = 5*x^3 + 1/x + 1/(x-1) + exp(x)/(3*x^2 + 1)
sage: sage: ff = FFPD(f)
sage: sage: %timeit -n 1 -r 1 ff.univariate_decomposition()
1 loops, best of 1: 6.07 ms per loop
sage: sage: %timeit -n 1 -r 1 f.partial_fraction()
1 loops, best of 1: 9.7 ms per loop
</pre></li></ul><p>
I've added a not in the docstring of the method, where this is pointed out.
</p>
TicketgitSun, 31 Jan 2016 14:29:50 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:109
https://trac.sagemath.org/ticket/10519#comment:109
<ul>
<li><strong>commit</strong>
changed from <em>1f21184f37244dbdf0395c75f474e4ca9429557f</em> to <em>ae42ad6539aaf99a7986a2bb1c2f018262296d5b</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="http://git.sagemath.org/sage.git/commit/?id=cc616fb5d79d8b561015cf7777db49be38de0115"><span class="icon"></span>cc616fb</a></td><td><code>Trac #10519: fix doctest (simplify symbolic coefficients)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ae42ad6539aaf99a7986a2bb1c2f018262296d5b"><span class="icon"></span>ae42ad6</a></td><td><code>Trac #10519: partial revert of commit 137f3c and follow-up ticket</code>
</td></tr></table>
TicketdkrennSun, 31 Jan 2016 14:30:48 GMT
https://trac.sagemath.org/ticket/10519#comment:110
https://trac.sagemath.org/ticket/10519#comment:110
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:109" title="Comment 109">git</a>:
</p>
<blockquote class="citation">
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ae42ad6539aaf99a7986a2bb1c2f018262296d5b"><span class="icon"></span>ae42ad6</a></td><td><code>Trac #10519: partial revert of commit 137f3c and follow-up ticket</code>
</td></tr></table>
</blockquote>
<p>
See <a class="new ticket" href="https://trac.sagemath.org/ticket/19989" title="defect: asymptotics for multivariate generating functions: zero division error ... (new)">#19989</a> for this issue.
</p>
TicketdkrennSun, 31 Jan 2016 14:38:43 GMTstatus, cc changed
https://trac.sagemath.org/ticket/10519#comment:111
https://trac.sagemath.org/ticket/10519#comment:111
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>cc</strong>
<em>behackl</em> added
</li>
</ul>
<p>
Ticket is again at <code>needs_review</code>. I've incorporated the comments above; please cross-review these changes.
</p>
<p>
A short summary (since the ticket is open for over 5 years now): The original code was posted by Alex Raichev. I've reviewed this code about two years ago. Mathematically it is fine, but it had to be included into <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a> better (see comments above). This is now done and from my part a positive review. However, since I made quite a lot of changes, these changes need a cross-review. Note that I've now marked the code as <a class="ext-link" href="http://doc.sagemath.org/html/en/developer/coding_in_python.html#experimental-unstable-code"><span class="icon"></span>experimental</a>.
</p>
TickettscrimSun, 31 Jan 2016 14:47:41 GMT
https://trac.sagemath.org/ticket/10519#comment:112
https://trac.sagemath.org/ticket/10519#comment:112
<p>
A few very quick thoughts from a very quick glance:
</p>
<ul><li>Remove input/output blocks that have nothing. IMO this adds clutter to the docstrings.
</li><li>How experimental/unstable do you think this code really is?
</li></ul>
TicketgitSun, 31 Jan 2016 14:54:00 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:113
https://trac.sagemath.org/ticket/10519#comment:113
<ul>
<li><strong>commit</strong>
changed from <em>ae42ad6539aaf99a7986a2bb1c2f018262296d5b</em> to <em>2056884ccadd2eee615c612cb137b5dd7d7a94f2</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="http://git.sagemath.org/sage.git/commit/?id=2056884ccadd2eee615c612cb137b5dd7d7a94f2"><span class="icon"></span>2056884</a></td><td><code>Trac #10519: remove "Nothing" input/output blocks</code>
</td></tr></table>
TicketdkrennSun, 31 Jan 2016 15:00:06 GMT
https://trac.sagemath.org/ticket/10519#comment:114
https://trac.sagemath.org/ticket/10519#comment:114
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:112" title="Comment 112">tscrim</a>:
</p>
<blockquote class="citation">
<ul><li>Remove input/output blocks that have nothing. IMO this adds clutter to the docstrings.
</li></ul></blockquote>
<p>
Done.
</p>
<blockquote class="citation">
<ul><li>How experimental/unstable do you think this code really is?
</li></ul></blockquote>
<p>
I think the code is quite stable; AFAIK, Mark Wilson (one of the authors of the corresponding theory behind this) uses basically this code (not from the ticket, but a fork) for his research now for some years.
I've marked it "experimental" since it is a new module and the interplay of the code with the whole <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>-infrastructure was changed on this ticket. This is the part, where there is not that much experience, but this will change once it is in <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>. If you feel that "experimental" should be remove...I'm fine with any choice.
</p>
TickettscrimSun, 31 Jan 2016 15:46:02 GMT
https://trac.sagemath.org/ticket/10519#comment:115
https://trac.sagemath.org/ticket/10519#comment:115
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:114" title="Comment 114">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:112" title="Comment 112">tscrim</a>:
</p>
<blockquote class="citation">
<ul><li>How experimental/unstable do you think this code really is?
</li></ul></blockquote>
<p>
I think the code is quite stable; AFAIK, Mark Wilson (one of the authors of the corresponding theory behind this) uses basically this code (not from the ticket, but a fork) for his research now for some years.
I've marked it "experimental" since it is a new module and the interplay of the code with the whole <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>-infrastructure was changed on this ticket. This is the part, where there is not that much experience, but this will change once it is in <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>. If you feel that "experimental" should be remove...I'm fine with any choice.
</p>
</blockquote>
<p>
I would be surprised if the (public) API changed much, the current version of the code fits in with the category framework, and it has been/will be (well) reviewed. So I feel that it should not be considered experimental.
</p>
<p>
We should also update the references (I can do this on my review changes).
</p>
TicketgitSun, 31 Jan 2016 15:54:06 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:116
https://trac.sagemath.org/ticket/10519#comment:116
<ul>
<li><strong>commit</strong>
changed from <em>2056884ccadd2eee615c612cb137b5dd7d7a94f2</em> to <em>e3668f46714d262e20b7ab7adc514ac6f8e09cd8</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="http://git.sagemath.org/sage.git/commit/?id=e3668f46714d262e20b7ab7adc514ac6f8e09cd8"><span class="icon"></span>e3668f4</a></td><td><code>Trac #10519: Revert "mark class as experimental"</code>
</td></tr></table>
TicketdkrennSun, 31 Jan 2016 15:57:31 GMT
https://trac.sagemath.org/ticket/10519#comment:117
https://trac.sagemath.org/ticket/10519#comment:117
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:115" title="Comment 115">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:114" title="Comment 114">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:112" title="Comment 112">tscrim</a>:
</p>
<blockquote class="citation">
<ul><li>How experimental/unstable do you think this code really is?
</li></ul></blockquote>
<p>
[...]
</p>
</blockquote>
<p>
I would be surprised if the (public) API changed much, the current version of the code fits in with the category framework, and it has been/will be (well) reviewed. So I feel that it should not be considered experimental.
</p>
</blockquote>
<p>
Good; I did <code>git revert</code> on the corresponding commit.
</p>
TicketdkrennSun, 31 Jan 2016 15:57:57 GMT
https://trac.sagemath.org/ticket/10519#comment:118
https://trac.sagemath.org/ticket/10519#comment:118
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:115" title="Comment 115">tscrim</a>:
</p>
<blockquote class="citation">
<p>
We should also update the references (I can do this on my review changes).
</p>
</blockquote>
<p>
Yes, we should. Thanks for doing.
</p>
TicketcheubergSun, 14 Feb 2016 15:05:31 GMTdescription, milestone changed
https://trac.sagemath.org/ticket/10519#comment:119
https://trac.sagemath.org/ticket/10519#comment:119
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10519?action=diff&version=119">diff</a>)
</li>
<li><strong>milestone</strong>
changed from <em>sage-6.4</em> to <em>sage-7.1</em>
</li>
</ul>
TicketgitMon, 15 Feb 2016 04:19:41 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:120
https://trac.sagemath.org/ticket/10519#comment:120
<ul>
<li><strong>commit</strong>
changed from <em>e3668f46714d262e20b7ab7adc514ac6f8e09cd8</em> to <em>badafce7372353c16c1b6ca658c183957ac2021b</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="http://git.sagemath.org/sage.git/commit/?id=64c081ad91046c6ee678cdd0ad625d96e03b5ccc"><span class="icon"></span>64c081a</a></td><td><code>Merge branch 'public/combinat/10519' of trac.sagemath.org:sage into public/combinat/10519</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=badafce7372353c16c1b6ca658c183957ac2021b"><span class="icon"></span>badafce</a></td><td><code>Reviewer code and documentation changes.</code>
</td></tr></table>
TickettscrimMon, 15 Feb 2016 04:32:29 GMTreviewer changed
https://trac.sagemath.org/ticket/10519#comment:121
https://trac.sagemath.org/ticket/10519#comment:121
<ul>
<li><strong>reviewer</strong>
changed from <em>Daniel Krenn, David Loeffler</em> to <em>Daniel Krenn, David Loeffler, Travis Scrimshaw</em>
</li>
</ul>
<p>
So I made my pass through the code (finally). I think it looks good with my changes:
</p>
<ul><li>I added a <code>__classcall_private__</code> to parse the input for <code>UniqueRepresentation</code>.
</li><li>I moved the (other) static methods to separate functions within the module since (IMO) they were not strongly associated with the <code>FractionWithFactoredDenominatorRing</code> class. It also allows their doc to be included by default (since they do not have to be _hidden) and allows them to potentially be used outside of the class.
</li><li>I moved around some of the documentation within methods as I feel the descriptive information should come before the <code>INPUT</code>/<code>OUTPUT</code> blocks. The input signature of the method is the first set of information given by <code>?</code> and the documentation.
</li><li>I moved more common imports to the start. There is no danger of circular imports when the file in question is not imported into the global namespace (it is also faster, easier to maintain in the long term, and I'm not as paranoid about these thing).
</li><li>Replaced some Sage implementations with there standard python equivalents.
</li><li>Other misc changes.
</li></ul><p>
If you're happy with my changes, then you can set a positive review.
</p>
<p>
Perhaps for a followup, we should figure out what the best way is to expose this functionality into the global namespace. Should we just do a lazy import of <code>FractionWithFactoredDenominatorRing</code>?
</p>
TicketgitMon, 15 Feb 2016 12:47:38 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:122
https://trac.sagemath.org/ticket/10519#comment:122
<ul>
<li><strong>commit</strong>
changed from <em>badafce7372353c16c1b6ca658c183957ac2021b</em> to <em>a30a18a230a1f77fac00e636b779df8f571eda62</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="http://git.sagemath.org/sage.git/commit/?id=a30a18a230a1f77fac00e636b779df8f571eda62"><span class="icon"></span>a30a18a</a></td><td><code>Trac #10519: two small cross-reviewing changes</code>
</td></tr></table>
TicketdkrennMon, 15 Feb 2016 12:57:36 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:123
https://trac.sagemath.org/ticket/10519#comment:123
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:121" title="Comment 121">tscrim</a>:
</p>
<blockquote class="citation">
<p>
So I made my pass through the code (finally).
</p>
</blockquote>
<p>
Thank you very much.
</p>
<blockquote class="citation">
<ul><li>I added a <code>__classcall_private__</code> to parse the input for <code>UniqueRepresentation</code>.
</li></ul></blockquote>
<p>
Good; checked.
</p>
<blockquote class="citation">
<ul><li>I moved the (other) static methods to separate functions within the module since (IMO) they were not strongly associated with the <code>FractionWithFactoredDenominatorRing</code> class. It also allows their doc to be included by default (since they do not have to be _hidden) and allows them to potentially be used outside of the class.
</li></ul></blockquote>
<p>
I agree that this is a good idea.
</p>
<blockquote class="citation">
<ul><li>I moved around some of the documentation within methods as I feel the descriptive information should come before the <code>INPUT</code>/<code>OUTPUT</code> blocks. The input signature of the method is the first set of information given by <code>?</code> and the documentation.
</li></ul></blockquote>
<p>
Ok, looks good.
</p>
<blockquote class="citation">
<ul><li>I moved more common imports to the start. There is no danger of circular imports when the file in question is not imported into the global namespace (it is also faster, easier to maintain in the long term, and I'm not as paranoid about these thing).
</li></ul></blockquote>
<p>
I'm fine with it. Just out of curiosity, why is this faster?
</p>
<blockquote class="citation">
<ul><li>Replaced some Sage implementations with there standard python equivalents.
</li></ul></blockquote>
<p>
Ok.
</p>
<blockquote class="citation">
<ul><li>Other misc changes.
</li></ul></blockquote>
<p>
I've checked the diffs (all 200 hunks); LGTM
</p>
<blockquote class="citation">
<p>
If you're happy with my changes, then you can set a positive review.
</p>
</blockquote>
<p>
I've added one doctest to make sage-coverage happier and removed one full-stop.
</p>
<blockquote class="citation">
<p>
Perhaps for a followup, we should figure out what the best way is to expose this functionality into the global namespace. Should we just do a lazy import of <code>FractionWithFactoredDenominatorRing</code>?
</p>
</blockquote>
<p>
Maybe. (A lazy import is indeed suitable.)
</p>
<p>
I set this to positive; all tests pass, doc builds and looks good.
</p>
TickettscrimMon, 15 Feb 2016 15:51:16 GMT
https://trac.sagemath.org/ticket/10519#comment:124
https://trac.sagemath.org/ticket/10519#comment:124
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:123" title="Comment 123">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:121" title="Comment 121">tscrim</a>:
</p>
<blockquote class="citation">
<ul><li>I moved more common imports to the start. There is no danger of circular imports when the file in question is not imported into the global namespace (it is also faster, easier to maintain in the long term, and I'm not as paranoid about these thing).
</li></ul></blockquote>
<p>
I'm fine with it. Just out of curiosity, why is this faster?
</p>
</blockquote>
<p>
Every time the function gets called, Python needs to check to see if it has imported the object. While this is more on the level of micro-optimization, it can matter in those methods used in a tight loop.
</p>
<blockquote class="citation">
<p>
I've added one doctest to make sage-coverage happier and removed one full-stop.
</p>
</blockquote>
<p>
Whoops. Good catch.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Perhaps for a followup, we should figure out what the best way is to expose this functionality into the global namespace. Should we just do a lazy import of <code>FractionWithFactoredDenominatorRing</code>?
</p>
</blockquote>
<p>
Maybe. (A lazy import is indeed suitable.)
</p>
<p>
I set this to positive; all tests pass, doc builds and looks good.
</p>
</blockquote>
<p>
We also should remove the deprecation warnings in <code>_element_constructor_</code> on said followup as I forgot to remove them here.
</p>
TicketdkrennMon, 15 Feb 2016 16:10:05 GMT
https://trac.sagemath.org/ticket/10519#comment:125
https://trac.sagemath.org/ticket/10519#comment:125
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:124" title="Comment 124">tscrim</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:123" title="Comment 123">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:121" title="Comment 121">tscrim</a>:
</p>
<blockquote class="citation">
<ul><li>I moved more common imports to the start. There is no danger of circular imports when the file in question is not imported into the global namespace (it is also faster, easier to maintain in the long term, and I'm not as paranoid about these thing).
</li></ul></blockquote>
<p>
I'm fine with it. Just out of curiosity, why is this faster?
</p>
</blockquote>
<p>
Every time the function gets called, Python needs to check to see if it has imported the object. While this is more on the level of micro-optimization, it can matter in those methods used in a tight loop.
</p>
</blockquote>
<p>
Ok, thank you for your explanation.
</p>
<blockquote class="citation">
<p>
We also should remove the deprecation warnings in <code>_element_constructor_</code> on said followup as I forgot to remove them here.
</p>
</blockquote>
<p>
Not a strong preference, but actually, I am for keeping the deprecation for some time. The code of this ticket has been around for about 5 years outside of SageMath (more or less as <a class="ext-link" href="https://github.com/araichev/amgf"><span class="icon"></span>https://github.com/araichev/amgf</a>), and there the deprecated parameters were used. People who are using this package will transition to the one soon to be included in SageMath, but I think some hints on what to change could be very helpful.
</p>
TickettscrimMon, 15 Feb 2016 16:17:38 GMT
https://trac.sagemath.org/ticket/10519#comment:126
https://trac.sagemath.org/ticket/10519#comment:126
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:125" title="Comment 125">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:124" title="Comment 124">tscrim</a>:
</p>
<blockquote class="citation">
<p>
We also should remove the deprecation warnings in <code>_element_constructor_</code> on said followup as I forgot to remove them here.
</p>
</blockquote>
<p>
Not a strong preference, but actually, I am for keeping the deprecation for some time. The code of this ticket has been around for about 5 years outside of SageMath (more or less as <a class="ext-link" href="https://github.com/araichev/amgf"><span class="icon"></span>https://github.com/araichev/amgf</a>), and there the deprecated parameters were used. People who are using this package will transition to the one soon to be included in SageMath, but I think some hints on what to change could be very helpful.
</p>
</blockquote>
<p>
Fair enough. Although I think because it had not been reviewed and included in Sage, it is fair game to change the API. However, I don't have a stake in this, but it is something we will eventually want to remove.
</p>
TicketvbraunMon, 15 Feb 2016 21:16:31 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:127
https://trac.sagemath.org/ticket/10519#comment:127
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
There are some tests that depend on ordering of symbolic variables:
</p>
<pre class="wiki">sage -t --long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 98, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions
Failed example:
s
Expected:
[{y: 1, x: 1}]
Got:
[{x: 1, y: 1}]
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 2828, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.FractionWithFactoredDenominator.smooth_critical_ideal
Failed example:
F.smooth_critical_ideal(alpha)
Expected:
Ideal (y^2 + 2*a1/a2*y - 1, x + ((-a2)/a1)*y + (-a1 + a2)/a1) of
Multivariate Polynomial Ring in x, y over Fraction Field of
Multivariate Polynomial Ring in a1, a2 over Rational Field
Got:
Ideal (y^2 + 2*a1/a2*y - 1, x + ((-a2)/a1)*y + (a2 - a1)/a1) of Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in a2, a1 over Rational Field
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 4373, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.coerce_point
Failed example:
p
Expected:
{y: 7/8, x: 1}
Got:
{x: 1, y: 7/8}
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 4375, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.coerce_point
Failed example:
for k in sorted(p.keys()):
print k, k.parent()
Expected:
y Symbolic Ring
x Symbolic Ring
Got:
x Symbolic Ring
y Symbolic Ring
**********************************************************************
3 items had failures:
1 of 77 in sage.rings.asymptotic.asymptotics_multivariate_generating_functions
1 of 16 in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.FractionWithFactoredDenominator.smooth_critical_ideal
2 of 11 in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.coerce_point
[809 tests, 4 failures, 74.86 s]
</pre>
TicketdkrennTue, 16 Feb 2016 07:48:51 GMT
https://trac.sagemath.org/ticket/10519#comment:128
https://trac.sagemath.org/ticket/10519#comment:128
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:127" title="Comment 127">vbraun</a>:
</p>
<blockquote class="citation">
<pre class="wiki">sage -t --long src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py
**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 98, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions
Failed example:
s
Expected:
[{y: 1, x: 1}]
Got:
[{x: 1, y: 1}]
</pre></blockquote>
<p>
Hmmm...I thought there is something like a display-hook, which catches such things (orderings in dicts and sets).
</p>
<blockquote class="citation">
<pre class="wiki">**********************************************************************
File "src/sage/rings/asymptotic/asymptotics_multivariate_generating_functions.py", line 4373, in sage.rings.asymptotic.asymptotics_multivariate_generating_functions.coerce_point
Failed example:
p
Expected:
{y: 7/8, x: 1}
Got:
{x: 1, y: 7/8}
</pre></blockquote>
<p>
The same here...
</p>
<p>
For the other issues: I am confident to fix.
</p>
TicketgitTue, 16 Feb 2016 08:05:48 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:129
https://trac.sagemath.org/ticket/10519#comment:129
<ul>
<li><strong>commit</strong>
changed from <em>a30a18a230a1f77fac00e636b779df8f571eda62</em> to <em>f58efc98ea4a1781e64250af5fe08339eaab47c8</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="http://git.sagemath.org/sage.git/commit/?id=f58efc98ea4a1781e64250af5fe08339eaab47c8"><span class="icon"></span>f58efc9</a></td><td><code>Trac #10519: fix sorting of variables (make it platform independent)</code>
</td></tr></table>
TicketgitTue, 16 Feb 2016 08:24:43 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:130
https://trac.sagemath.org/ticket/10519#comment:130
<ul>
<li><strong>commit</strong>
changed from <em>f58efc98ea4a1781e64250af5fe08339eaab47c8</em> to <em>85c1f3793979b44878c9e729d2e97b29258f2a7c</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="http://git.sagemath.org/sage.git/commit/?id=c37497f678252600aa20e54abaf5d5f33113a32e"><span class="icon"></span>c37497f</a></td><td><code>Trac #10519: more fixes to make platform-independent</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=85c1f3793979b44878c9e729d2e97b29258f2a7c"><span class="icon"></span>85c1f37</a></td><td><code>Trac #10519: another fix; not really nice... :(</code>
</td></tr></table>
TicketvbraunTue, 16 Feb 2016 08:25:30 GMT
https://trac.sagemath.org/ticket/10519#comment:131
https://trac.sagemath.org/ticket/10519#comment:131
<p>
The displayhook sorts but x, y have no particular order
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c37497f678252600aa20e54abaf5d5f33113a32e"><span class="icon"></span>c37497f</a></td><td><code>Trac #10519: more fixes to make platform-independent</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=85c1f3793979b44878c9e729d2e97b29258f2a7c"><span class="icon"></span>85c1f37</a></td><td><code>Trac #10519: another fix; not really nice... :(</code>
</td></tr></table>
TicketdkrennTue, 16 Feb 2016 08:27:40 GMT
https://trac.sagemath.org/ticket/10519#comment:132
https://trac.sagemath.org/ticket/10519#comment:132
<p>
I've now fixed all (at least I hope I got all) platform-dependent. Can someone please check with other platform than Linux, x86_64 on Intel i5-4690?
</p>
TicketdkrennTue, 16 Feb 2016 08:28:27 GMT
https://trac.sagemath.org/ticket/10519#comment:133
https://trac.sagemath.org/ticket/10519#comment:133
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:131" title="Comment 131">vbraun</a>:
</p>
<blockquote class="citation">
<p>
The displayhook sorts but x, y have no particular order
</p>
</blockquote>
<p>
Oh, I see (I thought, that display hooks sort by <code>str</code> in such a case). Thanks.
</p>
TickettscrimTue, 16 Feb 2016 15:42:55 GMT
https://trac.sagemath.org/ticket/10519#comment:134
https://trac.sagemath.org/ticket/10519#comment:134
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:132" title="Comment 132">dkrenn</a>:
</p>
<blockquote class="citation">
<p>
I've now fixed all (at least I hope I got all) platform-dependent. Can someone please check with other platform than Linux, x86_64 on Intel i5-4690?
</p>
</blockquote>
<p>
I didn't see a difference from the original doctests on my laptop, so I can't give an explicit check. However, from looking over the doctestvs and Volker's failures, this seems to do the trick. Although I don't like the changes on 85c1f37. Instead, I think you should just explicitly check that the dictionary of solutions is the correct one:
</p>
<pre class="wiki">sage: s == [{SR(x): 1, SR(y): 1}]
True
</pre>
TicketgitTue, 16 Feb 2016 15:59:27 GMTcommit changed
https://trac.sagemath.org/ticket/10519#comment:135
https://trac.sagemath.org/ticket/10519#comment:135
<ul>
<li><strong>commit</strong>
changed from <em>85c1f3793979b44878c9e729d2e97b29258f2a7c</em> to <em>3182c5b134f527de94a3728e7394e7fc27014c3e</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="http://git.sagemath.org/sage.git/commit/?id=59792e28311608920bb116bef8ec673c12cb6dfb"><span class="icon"></span>59792e2</a></td><td><code>Revert "Trac #10519: another fix; not really nice... :("</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3182c5b134f527de94a3728e7394e7fc27014c3e"><span class="icon"></span>3182c5b</a></td><td><code>Trac #10519: better fix for previous issue</code>
</td></tr></table>
TicketdkrennTue, 16 Feb 2016 16:01:52 GMT
https://trac.sagemath.org/ticket/10519#comment:136
https://trac.sagemath.org/ticket/10519#comment:136
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10519#comment:134" title="Comment 134">tscrim</a>:
Although I don't like the changes on 85c1f37. Instead, I think you should just explicitly check that the dictionary of solutions is the correct one:
</p>
<blockquote class="citation">
<pre class="wiki">sage: s == [{SR(x): 1, SR(y): 1}]
True
</pre></blockquote>
<p>
Reverted and changed; indeed much nicer.
</p>
TicketdkrennTue, 16 Feb 2016 16:02:03 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:137
https://trac.sagemath.org/ticket/10519#comment:137
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TickettscrimTue, 16 Feb 2016 16:02:25 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:138
https://trac.sagemath.org/ticket/10519#comment:138
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Thanks!
</p>
TicketgitTue, 16 Feb 2016 16:22:36 GMTstatus, commit changed
https://trac.sagemath.org/ticket/10519#comment:139
https://trac.sagemath.org/ticket/10519#comment:139
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
changed from <em>3182c5b134f527de94a3728e7394e7fc27014c3e</em> to <em>a951f08123d038f97b809095208bb04c22671a5e</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a951f08123d038f97b809095208bb04c22671a5e"><span class="icon"></span>a951f08</a></td><td><code>Trac #10519: trivial ReST error</code>
</td></tr></table>
TickettscrimTue, 16 Feb 2016 16:24:34 GMTstatus changed
https://trac.sagemath.org/ticket/10519#comment:140
https://trac.sagemath.org/ticket/10519#comment:140
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketvbraunWed, 17 Feb 2016 21:12:36 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/10519#comment:141
https://trac.sagemath.org/ticket/10519#comment:141
<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/combinat/10519</em> to <em>a951f08123d038f97b809095208bb04c22671a5e</em>
</li>
</ul>
Ticket