Sage: Ticket #9400: modify the NumberField constructor to pass in optional integer B such that all the internal pari routines will replace the discriminant by its gcd with B, making some things massively faster.
https://trac.sagemath.org/ticket/9400
<p>
Also:
</p>
<ul><li>ReSTify residue class fields
</li></ul><ul><li>massively optimize reduction modulo a prime (e.g., the first interesting example I tried, I got a speedup of a factor of 500,000! Yes half a million times faster!).
</li></ul><p>
Parts of this patch have been moved to <a class="closed ticket" href="https://trac.sagemath.org/ticket/9764" title="enhancement: Change hashing and printing for NumberFieldIdeals (closed: fixed)">#9764</a>.
</p>
<p>
Apply only <code>9400_combined.patch</code> and apply it on top of the patches from <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a>. See also <a class="ext-link" href="http://wiki.sagemath.org/NewPARI"><span class="icon"></span>http://wiki.sagemath.org/NewPARI</a>.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/9400
Trac 1.1.6wasThu, 01 Jul 2010 05:54:04 GMTdescription changed
https://trac.sagemath.org/ticket/9400#comment:1
https://trac.sagemath.org/ticket/9400#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9400?action=diff&version=1">diff</a>)
</li>
</ul>
TicketwasThu, 01 Jul 2010 05:56:20 GMTdescription changed
https://trac.sagemath.org/ticket/9400#comment:2
https://trac.sagemath.org/ticket/9400#comment:2
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9400?action=diff&version=2">diff</a>)
</li>
</ul>
TicketwasThu, 01 Jul 2010 06:48:16 GMT
https://trac.sagemath.org/ticket/9400#comment:3
https://trac.sagemath.org/ticket/9400#comment:3
<p>
Example of what this makes possible:
</p>
<pre class="wiki">~wstein/bin/sagedb
----------------------------------------------------------------------
| Sage Version 4.4.4, Release Date: 2010-06-23 |
| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage: R.<x> = QQ[]
sage: f = (x^20 - 3*x^19 - 29*x^18 + 91*x^17 + 338*x^16 - 1130*x^15 -
2023*x^14 +
....: 7432*x^13 + 6558*x^12 - 28021*x^11 - 10909*x^10 + 61267*x^9 + 6954*x^8 -
....: 74752*x^7 + 1407*x^6 + 46330*x^5 - 1087*x^4 - 12558*x^3 - 942*x^2 +
....: 960*x + 148)
sage: K.<a> = NumberField(f^2+2, maximize_at_primes=[2])
sage: K.degree()
40
sage: time z = K.factor(2)
CPU times: user 0.59 s, sys: 0.01 s, total: 0.60 s
Wall time: 0.60 s
sage: time k = z[0][0].residue_field()
CPU times: user 1.68 s, sys: 0.03 s, total: 1.71 s
Wall time: 1.98 s
sage: time k(a^3+3)
CPU times: user 0.01 s, sys: 0.00 s, total: 0.01 s
Wall time: 0.01 s
abar^3 + 1
</pre>
TicketwasWed, 11 Aug 2010 22:12:36 GMTstatus changed
https://trac.sagemath.org/ticket/9400#comment:4
https://trac.sagemath.org/ticket/9400#comment:4
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketjdemeyerThu, 12 Aug 2010 06:23:39 GMT
https://trac.sagemath.org/ticket/9400#comment:5
https://trac.sagemath.org/ticket/9400#comment:5
<p>
It is clear from a very first look at the patch that this will massively conflict with <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a> (why did nobody point this out to me earlier???). Personally, I would prefer to postpone the last two points of the description, i.e.
</p>
<ul><li>implement hashing for number field ideals that isn't a stupid string repr, hence vastly faster
</li></ul><ul><li>make number field ideals *not* print in reduced form; this will look uglier, but is massively faster and more sensible for any real applications, as people learned constantly at sage days 22.
</li></ul><p>
and do them after <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a> is merged (I also want to change the hashing of PARI objects, see <a class="closed ticket" href="https://trac.sagemath.org/ticket/9667" title="enhancement: Use PARI's hash_GEN() for gen.__hash__ (closed: duplicate)">#9667</a>). Also, I'm personally not completely convinced about the best way to print <a class="missing wiki">NumberFieldIdeals?</a> (see also my post at sage-devel).
</p>
TicketwasThu, 12 Aug 2010 11:15:31 GMT
https://trac.sagemath.org/ticket/9400#comment:6
https://trac.sagemath.org/ticket/9400#comment:6
<blockquote class="citation">
<p>
It is clear from a very first look at the patch that this will massively conflict with <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a> (why
</p>
</blockquote>
<p>
I'm sure this will be easy to merge with <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a>. It's probably best to get 9343 in first, since it is much more difficult, then rebase the current patch against it.
</p>
<blockquote class="citation">
<p>
I would prefer to postpone: hashing, printing
</p>
</blockquote>
<p>
The current hashing and printing setup is complete and total crap, and needs to be removed ASAP. It renders number fields completely useless for any nontrivial applications that involve prime ideals.
</p>
<blockquote class="citation">
<p>
Also, I'm personally not completely convinced about the best way to print
</p>
</blockquote>
<p>
I saw that. I think the best solution is exactly what I implemented in the attached patch. Hash based on gens (trivial, as a hash should be). Print in reduced form only in small trivial cases (by default), but allow the user to easily up the cutoff if they want, for some reason.
</p>
TicketjdemeyerThu, 12 Aug 2010 17:16:44 GMT
https://trac.sagemath.org/ticket/9400#comment:7
https://trac.sagemath.org/ticket/9400#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:6" title="Comment 6">was</a>:
</p>
<blockquote class="citation">
<p>
I saw that. I think the best solution is exactly what I implemented in the attached patch. Hash based on gens (trivial, as a hash should be).
</p>
</blockquote>
<p>
Why not hash based on the HNF representation, as I propsed in <a class="closed ticket" href="https://trac.sagemath.org/ticket/9666" title="enhancement: Implement __hash__ for NumberFieldIdeal (closed: duplicate)">#9666</a>? I think this is more canonical than gens() and it is the native PARI format.
</p>
TicketjdemeyerThu, 12 Aug 2010 17:18:07 GMTcc set
https://trac.sagemath.org/ticket/9400#comment:8
https://trac.sagemath.org/ticket/9400#comment:8
<ul>
<li><strong>cc</strong>
<em>jdemeyer</em> added
</li>
</ul>
TicketwasSat, 14 Aug 2010 00:39:08 GMT
https://trac.sagemath.org/ticket/9400#comment:9
https://trac.sagemath.org/ticket/9400#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:7" title="Comment 7">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:6" title="Comment 6">was</a>:
</p>
<blockquote class="citation">
<p>
I saw that. I think the best solution is exactly what I implemented in the attached patch. Hash based on gens (trivial, as a hash should be).
</p>
</blockquote>
<p>
Why not hash based on the HNF representation, as I propsed in <a class="closed ticket" href="https://trac.sagemath.org/ticket/9666" title="enhancement: Implement __hash__ for NumberFieldIdeal (closed: duplicate)">#9666</a>?
I think this is more canonical than gens() and it is the native PARI format.
</p>
</blockquote>
<p>
CONS:
I think number fields will frequently be relative extensions, and we'll also consider ideals both in the maximal order and in suborders. Pari will likely have almost nothing to do with our general relative extensions (it's only currently used for relative extensions as a miserable crutch), and HNF doesn't apply for ideals in orders.
</p>
<p>
PROS:
When it works, hashing based on the HNF has the property that if I==J then hash(I) == hash(J). That's a very good property to have. With hashing of gens that fails.
</p>
<p>
Thus taken together, I'm OK with your proposal with the caveat that at some future time it has to be revisited for *relative* extensions.
</p>
TicketwasSat, 14 Aug 2010 00:57:06 GMTattachment set
https://trac.sagemath.org/ticket/9400
https://trac.sagemath.org/ticket/9400
<ul>
<li><strong>attachment</strong>
set to <em>trac_9400.patch</em>
</li>
</ul>
<p>
apply only this patch (which also addresses the referees issue with <span class="underline">hash</span>)
</p>
TicketwasSat, 14 Aug 2010 00:58:06 GMT
https://trac.sagemath.org/ticket/9400#comment:10
https://trac.sagemath.org/ticket/9400#comment:10
<p>
OK, new patch up that changes <span class="underline">hash</span>. It passes all doctests on sage.math.
</p>
TicketjdemeyerSat, 14 Aug 2010 22:36:27 GMT
https://trac.sagemath.org/ticket/9400#comment:11
https://trac.sagemath.org/ticket/9400#comment:11
<p>
The patch introduces an inconsistency:
</p>
<pre class="wiki">sage: K.<a> = NumberField(x^3-7)
sage: K.ideal(12*a + 5).factor()
(Fractional ideal (101, a - 8)) * (Fractional ideal (11, a + 5))^2
</pre><pre class="wiki">sage: K.<b> = NumberField(x^3-10001)
sage: L.ideal(b+1).factor()
(Fractional ideal (b + 1, 1667)) * (Fractional ideal (b + 1, 2)) * (Fractional ideal (b + 1, 3))
</pre><p>
Note how the integer is printed first in the first case but last in the second case (and personally I find it clearer when the integer is put first). Maybe the sorting and uniqueing should be done in the <a class="missing wiki">NumberFieldIdeal?</a> constructor instead of when the ideal is printed?
</p>
TicketwasSun, 15 Aug 2010 17:16:40 GMT
https://trac.sagemath.org/ticket/9400#comment:12
https://trac.sagemath.org/ticket/9400#comment:12
<blockquote class="citation">
<p>
Note how the integer is printed first in the first case but last in the second case (and personally I find it clearer when the integer is put first). Maybe the sorting and uniqueing should be done in the <a class="missing wiki">NumberFieldIdeal?</a> constructor instead of when the ideal is printed?
</p>
</blockquote>
<p>
Would that make any difference? The difference above is that in one case the number field has a very small discriminant (-1323), and in the other it doesn't (-2700540027). When the discriminant is small, then reduced generators are used for printing. A solution could be to apply the sorting and "uniquing" in both cases before printing -- right now it is only applied in the case of large discriminant.
</p>
TicketwasSun, 15 Aug 2010 17:46:31 GMT
https://trac.sagemath.org/ticket/9400#comment:13
https://trac.sagemath.org/ticket/9400#comment:13
<p>
Hi,
</p>
<p>
I retract my comment. The issue may be that sorting of elements of number fields is now useless. Observe:
</p>
<pre class="wiki">sage: L.<b> = NumberField(x^3-10001)
sage: b+1 < L(1667)
False
sage: L(1667) < b + 1
False
</pre><p>
Thus it doesn't matter *what* you do with sorting and uniquing the gens before or after -- there's no sensible ordering that will come out of this, unless elements of number fields get a total (non-algebraic) ordering. I thought they had one.
</p>
<p>
Oh, now I remember -- there is a *major bug* in the way elements of number fields are ordered. You can see this by looking at the code (I think Joel Mohler) wrote in number_field_element.pyx. I fixed this a few weeks ago as part of the patch bomb <a class="new ticket" href="https://trac.sagemath.org/ticket/9541" title="enhancement: optimize number field arithmetic using flint and singular (new)">#9541</a>.
</p>
<p>
So my advice is to not worry about sorting issues as part of *this* patch, but keep in mind that it is has to be fixed later. I've opened ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/9752" title="defect: sorting of number field elements is broken (closed: duplicate)">#9752</a> to fix this.
</p>
TicketjdemeyerMon, 16 Aug 2010 20:23:22 GMTmilestone changed
https://trac.sagemath.org/ticket/9400#comment:14
https://trac.sagemath.org/ticket/9400#comment:14
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.5.3</em> to <em>sage-4.6</em>
</li>
</ul>
<p>
Always using <code>gens()</code> for printing is also silly, because for larger degrees you can get something like:
</p>
<pre class="wiki">sage: x=polygen(QQ); K.<a> = NumberField(x^8+x-1);
sage: J1 = K.ideal(a+1); J2 = K.ideal(a+2); J = J1.intersection(J2)
sage: J
Fractional ideal (a^2 + 249, -a^7 + 125, a + 2, a^3 + 8, a^4 + 237, -a^7 - a^6 + 189, a^7 + a^6 + a^5 + 96, 253)
</pre><p>
In my opinion, the best way would be to use <code>idealtwoelt()</code> for large discriminants instead of simply printing all generators (or alternatively: reduce the generators using <code>idealtwoelt()</code> in <code>__init__</code>).
</p>
TicketwasWed, 18 Aug 2010 00:18:14 GMT
https://trac.sagemath.org/ticket/9400#comment:15
https://trac.sagemath.org/ticket/9400#comment:15
<pre class="wiki">On Tue, Aug 17, 2010 at 5:03 PM, Chan-Ho Kim <chanho.math> wrote:
> Dear William,
> I think that there is a bug on trac 9400 patch.
> My current SAGE is (SAGE 4.5.2 + trac 9400 patch only) in VM.
> When I use `maximize_at_prime,'
>
> K.<a> = NumberField(x^6 + 9*x^5 - 8410*x^4 - 88580*x^3 + 18705368*x^2 +
> 99820416*x - 12230355456, maximize_at_primes=[3]) ; K.primes_above(3)
> this decomposition in K works as you mentioned.
>
> However, in this ``small'' number field
>
> F.<a> = NumberField(x^3 - x^2 - 24*x + 32, maximize_at_primes=[3]) ;
> F.primes_above(3)
> the low precision error occurs if I add `maximize_at_primes=[3].'
That's not a bug in maximize_at_primes or finding the primes above 3. But it *is* an issue with *printing* the ideals out that it finds over 3. Evidently, when printing is_principal is called on each ideal currently, and this leads to a problem. This is not surprising, given that deciding whether or not an ideal is principal requires knowing the class group in general, and the equation order of F that you define above is not only deficient at 3. You need to also maximize at 2. See:
sage: F.<a> = NumberField(x^3 - x^2 - 24*x + 32, maximize_at_primes=[2,3])
sage: F.primes_above(3)
[Fractional ideal (-1/2*a^2 - 3/2*a + 5), Fractional ideal (-1/2*a^2 + 5/2*a - 1)]
So in short, this is not a bug. If you try to compute with number fields and pass in the maximize_at_primes option, certain things can't possibly work.
That said, I'm not a big fan of how ideals print. Maybe Jereon's suggestion -- just *always* print with the PARI 2-element representation -- is the way to go. That might get around this problem.
> BTW, I also have one more question:
> Can I add `maximize_at_prime=[p]' in `hecke_eigenvalue_field()'?
You'll have to dive in and start hacking at the source code of Sage to do that....
-- William
</pre>
TicketjdemeyerWed, 18 Aug 2010 21:20:49 GMT
https://trac.sagemath.org/ticket/9400#comment:16
https://trac.sagemath.org/ticket/9400#comment:16
<p>
I am not familiar with <code>_pari_</code> and <code>_pari_init_</code>, but why does <a class="missing wiki">NumberField?</a> need <code>_pari_init_</code>? Can't we add instead a <code>_pari_</code> method which returns <code>pari_nf()</code>?
</p>
TicketjdemeyerWed, 18 Aug 2010 21:26:11 GMT
https://trac.sagemath.org/ticket/9400#comment:17
https://trac.sagemath.org/ticket/9400#comment:17
<p>
Rebasing to <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a> will be easier if we seperate the "maximize_at_primes part" from the "printing and hashing of ideals" part. So I will cut this patch in two pieces.
</p>
TicketjdemeyerWed, 18 Aug 2010 22:02:15 GMTattachment set
https://trac.sagemath.org/ticket/9400
https://trac.sagemath.org/ticket/9400
<ul>
<li><strong>attachment</strong>
set to <em>9400_maximize_at_primes.patch</em>
</li>
</ul>
<p>
Patch for the "maximize at primes" part, rebased to sage-4.6.prealpha1 (see <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a>)
</p>
TicketjdemeyerWed, 18 Aug 2010 22:03:39 GMTdescription changed
https://trac.sagemath.org/ticket/9400#comment:18
https://trac.sagemath.org/ticket/9400#comment:18
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9400?action=diff&version=18">diff</a>)
</li>
</ul>
TicketjdemeyerFri, 20 Aug 2010 15:48:34 GMT
https://trac.sagemath.org/ticket/9400#comment:19
https://trac.sagemath.org/ticket/9400#comment:19
<p>
I attached a big reviewer patch (to be applied on top of 9400_maximize_at_primes.patch) changing:
</p>
<ul><li>the PARI functions <code>nfbasis()</code> and <code>nfinit()</code>: document and fix implementation.
</li><li>change <code>pari_nf()</code> to always use <code>integral_basis()</code> (this means that all the <code>maximize_at_primes</code> code can be moved out of <code>pari_nf()</code>).
</li><li>rename <code>_compute_integral_basis</code> to <code>_pari_integral_basis</code> and don't convert from PARI to Sage.
</li><li>make the code for <a class="missing wiki">CyclotomicField?</a> more analogous to NumberField_generic (such that only <code>_pari_integral_basis</code> needs to be specialized, not <code>integral_basis()</code>.
</li></ul><p>
Since the purpose of <code>_pari_init_()</code> is very unclear to me, I'm not sure what to do with that.
</p>
TicketjdemeyerSat, 21 Aug 2010 10:31:29 GMTattachment set
https://trac.sagemath.org/ticket/9400
https://trac.sagemath.org/ticket/9400
<ul>
<li><strong>attachment</strong>
set to <em>9400_jd_review.patch</em>
</li>
</ul>
<p>
Apply on top of 9400_maximize_at_primes.patc
</p>
TicketjdemeyerTue, 24 Aug 2010 19:34:30 GMTowner changed; keywords, author set
https://trac.sagemath.org/ticket/9400#comment:20
https://trac.sagemath.org/ticket/9400#comment:20
<ul>
<li><strong>keywords</strong>
<em>PARI</em> <em>number</em> <em>field</em> added
</li>
<li><strong>owner</strong>
changed from <em>davidloeffler</em> to <em>jdemeyer</em>
</li>
<li><strong>author</strong>
set to <em>William Stein, Jeroen Demeyer</em>
</li>
</ul>
TicketcremonaMon, 30 Aug 2010 16:09:10 GMT
https://trac.sagemath.org/ticket/9400#comment:21
https://trac.sagemath.org/ticket/9400#comment:21
<p>
Since this ismarked "needs review" could the authors clarify which patches are up for review? I assume it is
</p>
<pre class="wiki">9400_maximize_at_primes.patch
Patch for the "maximize at primes" part, rebased to sage-4.6.prealpha1 (see #9343)
</pre><p>
and
</p>
<pre class="wiki">9400_jd_review.patch
Apply on top of 9400_maximize_at_primes.patch
</pre><p>
and not the first one.
</p>
<p>
Secondly, since these patches have been merged into 4.6.prealpha3 which I have successfully built and tested, I reckon that the only (!) remaining task as a reviewer is to look at the code in the patch, with the additional explanations on the ticket, and approve (or otherwise, maybe) of it?
</p>
<p>
I will try to do this before I go away on Friday for a week. No promises...
</p>
TicketcremonaMon, 30 Aug 2010 16:33:13 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/9400#comment:22
https://trac.sagemath.org/ticket/9400#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>Jeroen Demeyer, William Stein, John Cremona</em>
</li>
</ul>
<p>
Looking at the 2nd and 3rd patches (separately) it looks very good to me. For the benefit of others reading this, the essence of these patches (apart from some ReSTification) is to expose some additional functionality from the pari library to Sage, so that some computations can be mande very much faster (a *lot* faster!).
</p>
<p>
I spotted a few minor issues in docstrings (numbers refer to the ones I see in the third patch):
</p>
<p>
6451: second 'it' --> 'the'
</p>
<p>
6466: should have a preceding #, or replace ":" by "::" and insert a blank line after and change the preceding "TESTS::" to "TESTS:"
</p>
<p>
6592: change "TESTS::" to "TESTS:"
</p>
<p>
and also when I rebuilt the docs after "sage -ba" (using 4.6.prealpha3) I got these ReST warnings:
</p>
<pre class="wiki">docstring of sage.libs.pari.gen:136: (WARNING/2) Literal block expected; none found.
</pre><pre class="wiki">docstring of sage.libs.pari.gen.gen.nfbasis:8: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
</pre><pre class="wiki">docstring of sage.libs.pari.gen.gen.nfinit:19: (WARNING/2) Literal block expected; none found.
</pre><p>
Modulo these, a positive review is on offer. Hence "needs work", but it is only trivial work.
</p>
TicketjdemeyerMon, 30 Aug 2010 21:30:02 GMT
https://trac.sagemath.org/ticket/9400#comment:23
https://trac.sagemath.org/ticket/9400#comment:23
<p>
Thanks John, I will take care of these issues when I have time.
</p>
TicketjdemeyerMon, 30 Aug 2010 21:33:13 GMTdescription changed
https://trac.sagemath.org/ticket/9400#comment:24
https://trac.sagemath.org/ticket/9400#comment:24
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9400?action=diff&version=24">diff</a>)
</li>
</ul>
TicketjdemeyerSun, 05 Sep 2010 12:41:26 GMTattachment set
https://trac.sagemath.org/ticket/9400
https://trac.sagemath.org/ticket/9400
<ul>
<li><strong>attachment</strong>
set to <em>9400_docfix.patch</em>
</li>
</ul>
<p>
Apply on top of previous 2 patches, small documentation fixes
</p>
TicketjdemeyerSun, 05 Sep 2010 13:28:43 GMTstatus, reviewer changed
https://trac.sagemath.org/ticket/9400#comment:25
https://trac.sagemath.org/ticket/9400#comment:25
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Jeroen Demeyer, William Stein, John Cremona</em> to <em>Jeroen Demeyer, John Cremona</em>
</li>
</ul>
TicketjdemeyerSun, 05 Sep 2010 16:34:45 GMTattachment set
https://trac.sagemath.org/ticket/9400
https://trac.sagemath.org/ticket/9400
<ul>
<li><strong>attachment</strong>
set to <em>9400_combined.patch</em>
</li>
</ul>
<p>
All 3 patches combined (apply only this patch)
</p>
TicketjdemeyerSun, 05 Sep 2010 16:39:18 GMTdescription changed
https://trac.sagemath.org/ticket/9400#comment:26
https://trac.sagemath.org/ticket/9400#comment:26
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/9400?action=diff&version=26">diff</a>)
</li>
</ul>
TicketjdemeyerSun, 05 Sep 2010 18:46:31 GMT
https://trac.sagemath.org/ticket/9400#comment:27
https://trac.sagemath.org/ticket/9400#comment:27
<p>
I did all the changes requested by the reviewer.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:22" title="Comment 22">cremona</a>:
</p>
<blockquote class="citation">
<pre class="wiki">docstring of sage.libs.pari.gen:136: (WARNING/2) Literal block expected; none found.
</pre></blockquote>
<p>
This belongs to <a class="closed ticket" href="https://trac.sagemath.org/ticket/9636" title="defect: Catch output from PARI in Sage (closed: fixed)">#9636</a> and is fixed there. The other warnings are fixed here.
</p>
TicketwasTue, 07 Sep 2010 16:14:42 GMTstatus changed
https://trac.sagemath.org/ticket/9400#comment:28
https://trac.sagemath.org/ticket/9400#comment:28
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Hi,
</p>
<p>
I tried to apply my patch to 4.5.2.rc0 and it works fine:
</p>
<pre class="wiki">adding trac_9400.patch to series file
applying trac_9400.patch
now at: trac_9400.patch
</pre><p>
I tried to apply your patch (9400_combined.patch) and there are a massive number of rejects:
</p>
<pre class="wiki">adding 9400_combined.patch to series file
applying 9400_combined.patch
patching file sage/libs/pari/gen.pyx
Hunk #1 succeeded at 6408 with fuzz 2 (offset -32 lines).
Hunk #4 FAILED at 6964
Hunk #5 FAILED at 6991
2 out of 7 hunks FAILED -- saving rejects to file sage/libs/pari/gen.pyx.rej
patching file sage/rings/number_field/number_field.py
Hunk #13 FAILED at 2870
Hunk #16 FAILED at 3727
2 out of 25 hunks FAILED -- saving rejects to file sage/rings/number_field/number_field.py.rej
patching file sage/rings/number_field/number_field_ideal_rel.py
Hunk #2 FAILED at 592
1 out of 2 hunks FAILED -- saving rejects to file sage/rings/number_field/number_field_ideal_rel.py.rej
patching file sage/rings/polynomial/polynomial_quotient_ring.py
Hunk #1 FAILED at 688
Hunk #2 FAILED at 1068
2 out of 2 hunks FAILED -- saving rejects to file sage/rings/polynomial/polynomial_quotient_ring.py.rej
patching file sage/rings/residue_field.pyx
Hunk #17 succeeded at 427 with fuzz 2 (offset 0 lines).
Hunk #18 succeeded at 444 with fuzz 2 (offset 0 lines).
Hunk #19 FAILED at 463
Hunk #26 succeeded at 572 with fuzz 1 (offset -1 lines).
Hunk #27 FAILED at 589
2 out of 48 hunks FAILED -- saving rejects to file sage/rings/residue_field.pyx.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 9400_combined.patch
</pre>
TicketjdemeyerTue, 07 Sep 2010 16:34:14 GMTstatus, description changed
https://trac.sagemath.org/ticket/9400#comment:29
https://trac.sagemath.org/ticket/9400#comment:29
<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/9400?action=diff&version=29">diff</a>)
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:28" title="Comment 28">was</a>:
</p>
<blockquote class="citation">
<p>
I tried to apply your patch (9400_combined.patch) and there are a massive number of rejects:
</p>
</blockquote>
<p>
It is meant to be applied after <a class="closed ticket" href="https://trac.sagemath.org/ticket/9343" title="enhancement: Upgrade PARI to svn snapshot 12577 - a pre-release of PARI 2.4.3. (closed: fixed)">#9343</a>, then it works fine.
</p>
TicketmpatelThu, 09 Sep 2010 11:12:00 GMT
https://trac.sagemath.org/ticket/9400#comment:30
https://trac.sagemath.org/ticket/9400#comment:30
<p>
John, do Jeroen's most recent changes look good to you?
</p>
TicketcremonaThu, 09 Sep 2010 14:39:47 GMT
https://trac.sagemath.org/ticket/9400#comment:31
https://trac.sagemath.org/ticket/9400#comment:31
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:30" title="Comment 30">mpatel</a>:
</p>
<blockquote class="citation">
<p>
John, do Jeroen's most recent changes look good to you?
</p>
</blockquote>
<p>
Sorry for delay -- Jeroen has nudged me on this and I'll look at it as soon as I can, but I'm at a conference this week...
</p>
TicketcremonaSat, 11 Sep 2010 16:31:03 GMT
https://trac.sagemath.org/ticket/9400#comment:32
https://trac.sagemath.org/ticket/9400#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:30" title="Comment 30">mpatel</a>:
</p>
<blockquote class="citation">
<p>
John, do Jeroen's most recent changes look good to you?
</p>
</blockquote>
<p>
The new combined patch does look good. It applies smoothly to 4.6.alpha0, and the docs (re)build with no warnings. I am currently doing a full test just to make sure. Will report back shortly.
</p>
TicketcremonaSat, 11 Sep 2010 16:43:08 GMTstatus changed
https://trac.sagemath.org/ticket/9400#comment:33
https://trac.sagemath.org/ticket/9400#comment:33
<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/9400#comment:32" title="Comment 32">cremona</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:30" title="Comment 30">mpatel</a>:
</p>
<blockquote class="citation">
<p>
John, do Jeroen's most recent changes look good to you?
</p>
</blockquote>
<p>
The new combined patch does look good. It applies smoothly to 4.6.alpha0, and the docs (re)build with no warnings. I am currently doing a full test just to make sure. Will report back shortly.
</p>
</blockquote>
<p>
All (long) tests pass -- positive review.
</p>
TicketmpatelWed, 15 Sep 2010 10:39:38 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/9400#comment:34
https://trac.sagemath.org/ticket/9400#comment:34
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-4.6.alpha1</em>
</li>
</ul>
TicketjdemeyerSun, 19 Sep 2010 10:47:26 GMT
https://trac.sagemath.org/ticket/9400#comment:35
https://trac.sagemath.org/ticket/9400#comment:35
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/9400#comment:34" title="Comment 34">mpatel</a>:
</p>
<p>
Note that I mentioned in this ticket (and on <a class="ext-link" href="http://groups.google.com/group/sage-devel/browse_thread/thread/ff993da70fc9f7ac"><span class="icon"></span>sage-devel</a>) that I do not understand <code>_pari_init_()</code>. I don't know if the reviewers do, so I'm a little bit worried about this.
</p>
Ticket