Sage: Ticket #2348: [with patch, positive review] Teach the gap interface about field extensions
https://trac.sagemath.org/ticket/2348
<p>
Define the following:
</p>
<pre class="wiki">sage: F = CyclotomicField(8)
sage: z = F.gen()
sage: a = z+1/z
sage: MS = MatrixSpace(F, 2, 2)
sage: g1 = MS([[1/a,1/a],[1/a,-1/a]])
sage: b = z^2
sage: g2 = MS([[1,0],[0,b]])
sage: g3 = MS([[b,0],[0,1]])
sage: G = MatrixGroup([g1,g2,g3])
</pre><p>
Then, one obtains a traceback by the attempt to see G:
</p>
<pre class="wiki">sage: G
<traceback removed>
<type 'exceptions.TypeError'>: Gap produced error output
Variable: 'zeta8' must have a value
executing Read("/home/king/.sage//temp/mpc739/6870//interface//tmp");
</pre><p>
Note that in fact <code>zeta8</code> is known:
</p>
<pre class="wiki">sage: G.base_ring().gen()
zeta8
</pre>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/2348
Trac 1.1.6SimonKingThu, 28 Feb 2008 19:16:26 GMT
https://trac.sagemath.org/ticket/2348#comment:1
https://trac.sagemath.org/ticket/2348#comment:1
<p>
Searching in <code>sage/groups/matrix_gps/matrix_group.py</code>, i found that <code>MatrixGroup([g1,g2,g3])</code> calls <code>MatrixGroup_gens.__init__</code>, which in turn calls <code>MatrixGroup_gap.__init__</code>. The latter has signature <code>(self, n, R, var='a')</code>.
</p>
<p>
In the above example, i guess <code>var</code> is supposed to be <code>'zeta8'</code>, and by consequence <code>G._var</code> should be <code>'zeta8'</code>. But it isn't, <code>G._var</code> will never be initialised with a value different from <code>'a'</code>.
</p>
<p>
I thought this might be a source of trouble. But unfortunately, it doesn't help to change <code>MatrixGroup_gens.__init__</code> accordingly.
</p>
<p>
By the way, it seems that the attribute <code>_var</code> is not used somewhere.
</p>
TicketwdjThu, 28 Feb 2008 19:47:37 GMT
https://trac.sagemath.org/ticket/2348#comment:2
https://trac.sagemath.org/ticket/2348#comment:2
<p>
This is my guess:
The problem is buried in _gap_init_, which behaves incorrectly for cyclotomics:
</p>
<p>
sage: F = GF(8,"z"); a = F.gen(); a._gap_init_()
'Z(8)<sup>1'
sage: F = <a class="missing wiki">CyclotomicField?</a>(8); a = F.gen(); a._gap_init_()
'zeta8'
</sup></p>
<p>
I don't know if fixing that will fix the problem though.
</p>
TicketwdjThu, 28 Feb 2008 19:55:29 GMT
https://trac.sagemath.org/ticket/2348#comment:3
https://trac.sagemath.org/ticket/2348#comment:3
<p>
I should have added that in the last line of
</p>
<pre class="wiki">sage: F = GF(8,"z"); a = F.gen(); a._gap_init_()
'Z(8)1'
sage: F = CyclotomicField?(8); a = F.gen(); a._gap_init_()
'zeta8'
</pre><p>
the output should be 'E(8)', or possibly 'E(8)<sup>1', since this
is the GAP notation for a primitive 8th root of unity.
</sup></p>
TicketSimonKingThu, 28 Feb 2008 19:59:03 GMTkeywords, component, summary changed; cc set
https://trac.sagemath.org/ticket/2348#comment:4
https://trac.sagemath.org/ticket/2348#comment:4
<ul>
<li><strong>keywords</strong>
<em>gap</em> <em>interface</em> added
</li>
<li><strong>cc</strong>
<em>was</em> <em>wdj</em> added
</li>
<li><strong>component</strong>
changed from <em>group_theory</em> to <em>interfaces</em>
</li>
<li><strong>summary</strong>
changed from <em>MatrixGroup over CyclotomicField is broken</em> to <em>gap(Matrix) and MatrixGroup over CyclotomicField are broken</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/2348#comment:2" title="Comment 2">wdj</a>:
</p>
<blockquote class="citation">
<p>
This is my guess:
The problem is buried in _gap_init_, which behaves incorrectly for cyclotomics:
</p>
</blockquote>
<p>
It seems to me that you are right that it is the gap interface. Therefore i change Summary, Component, and Keywords of the ticket, and send Cc to William.
</p>
<p>
In the above example, <code>G._repr_()</code> refers to <code>G.gens()</code>, and that is calling the gap interface for the matrices that have been used to define <code>G</code>.
</p>
<p>
While the following works,
</p>
<pre class="wiki">sage: F=CyclotomicField(8)
sage: N=Matrix(F,[[1,0],[0,1]])
sage: gap(N)
[ [ 1, 0 ], [ 0, 1 ] ]
</pre><p>
the following crashes,
</p>
<pre class="wiki">sage: M=Matrix(F,[[F.gen(),0],[0,F.gen()]])
sage: M
[zeta8 0]
[ 0 zeta8]
sage: gap(M)
</pre><p>
Again, <code>gap</code> complains that <code>'zeta8'</code> has no value.
</p>
<p>
For solving the problem with <code>MatrixGroup</code> i would appreciate if someone could explain to me
</p>
<ul><li>how one can introduce <code>zeta8</code> to <code>gap</code>
</li></ul><p>
or
</p>
<ul><li>how <code>F.gen()</code> should be called in order to be understood by <code>gap</code>
</li></ul>
TicketSimonKingFri, 29 Feb 2008 08:36:27 GMTkeywords, summary changed
https://trac.sagemath.org/ticket/2348#comment:5
https://trac.sagemath.org/ticket/2348#comment:5
<ul>
<li><strong>keywords</strong>
<em>extension</em> added; <em>interface</em> <em>matrix</em> <em>group</em> <em>cyclotomic</em> removed
</li>
<li><strong>summary</strong>
changed from <em>gap(Matrix) and MatrixGroup over CyclotomicField are broken</em> to <em>Teach the gap interface about field extensions</em>
</li>
</ul>
<p>
More and more it seems to me that the troubles come from gaps in the gap interface. So i think this ticket should really be focused on the interface.
</p>
<p>
In the above example, one has to tell <code>gap</code> that it shall create a field extension, namely the cyclotomic field. The following crashes:
</p>
<pre class="wiki">sage: F=CyclotomicField(8)
sage: gap(F)
---------------------------------------------------------------------------
<type 'exceptions.TypeError'> Traceback (most recent call last)
...
Syntax error: ; expected
$sage7:=Cyclotomic Field of order 8 and degree 4;;
^
Variable: 'of' must have a value
Variable: 'degree' must have a value
executing $sage7:=Cyclotomic Field of order 8 and degree 4;;
</pre><p>
Apparently, <code>gap(F)</code> is equivalent to <code>gap(str(F))</code>, which of course yields nonsense.
</p>
<p>
Instead, the <code>gap</code> related methods of the class <code>NumberField</code> and related classes should do something like that:
</p>
<pre class="wiki">sage: PR=PolynomialRing(F.base_field(),F.gen())
sage: pr=gap(PR)
sage: bf=gap(F.base_field())
sage: mp=gap(F.polynomial())
sage: f=bf.AlgebraicExtension(mp)
sage: f
<algebraic extension over the Rationals of degree 4>
</pre><p>
Now <code>f</code> is the <code>gap</code> version of <code>F</code>.
</p>
TicketwdjFri, 29 Feb 2008 11:51:38 GMT
https://trac.sagemath.org/ticket/2348#comment:6
https://trac.sagemath.org/ticket/2348#comment:6
<p>
I agree this needs fixing but, based on my reading of sage/interfaces/gap.py, I'm not sure how to do this. Do you have a suggestion William?
</p>
TicketSimonKingFri, 29 Feb 2008 23:26:49 GMT
https://trac.sagemath.org/ticket/2348#comment:7
https://trac.sagemath.org/ticket/2348#comment:7
<p>
I tried to figure out how the interfaces are used, and it seems to me that there will be much work to do.
</p>
<p>
Am i right that, if <code>X</code> is some Sage object, <code>gap(X)</code> sends the value of <code>X._gap_init_()</code> (which is a string) through the gap interface; and similarly <code>singular(X)</code> sends <code>X._singular_init_()</code>?
</p>
<p>
The problem is that <code>_gap_init_</code> and <code>_singular_init_</code> often can not be interpreted by gap or by Singular. Examples:
</p>
<pre class="wiki">sage: QQ._singular_init_()
'Rationals'
</pre><p>
This is ok, since <code>gap</code> knows what Rationals means. But:
</p>
<pre class="wiki">sage: QQ._singular_init_()
'Rational Field'
</pre><p>
This is something that Singular does not understand! In fact, for Singular the rationals do not exist, except as the base field of a polynomial ring. So i believe there should be a <code>NotImplementedError</code> in that case.
</p>
<pre class="wiki">sage: CyclotomicField(8)._gap_init_()
'Cyclotomic Field of order 8 and degree 4'
sage: CyclotomicField(8)._singular_init_()
'Cyclotomic Field of order 8 and degree 4'
</pre><p>
Neither gap nor Singular can interprete that string.
</p>
<p>
Something that seems inconsistent to me: the method <code>_singular_init_</code> is not always returning a string:
</p>
<pre class="wiki">sage: QQ[x]._gap_init_()
'PolynomialRing(Rationals, ["x"])'
sage: QQ[x]._singular_init_()
// characteristic : 0
// number of vars : 1
// block 1 : ordering lp
// : names x
// block 2 : ordering C
sage: type(QQ[x]._gap_init_())
<type 'str'>
sage: type(QQ[x]._singular_init_())
<class 'sage.interfaces.singular.SingularElement'>
</pre><p>
I found that in <code>sage/rings/number_field/number_field.py</code> there is no method <code>_singular_init_</code> or <code>_gap_init_</code>. Do you think it would solve the problem if one implements such methods?
</p>
TicketSimonKingSat, 01 Mar 2008 10:14:42 GMTattachment set
https://trac.sagemath.org/ticket/2348
https://trac.sagemath.org/ticket/2348
<ul>
<li><strong>attachment</strong>
set to <em>numberfields_gap.patch</em>
</li>
</ul>
<p>
This solves only a part of the problem
</p>
TicketSimonKingSat, 01 Mar 2008 10:50:02 GMT
https://trac.sagemath.org/ticket/2348#comment:8
https://trac.sagemath.org/ticket/2348#comment:8
<p>
I was just attaching a patch (relative to 2.10.3.rc0) that adds a <code>_gap_init_</code> method to <code>NumberField_generic</code> and may be part of a solution.
</p>
<p>
Sometimes <code>_singular_init_</code> does not return a string but a <a class="missing wiki">SingularElement?</a>. I don't know whether it is possible to define a number field in gap in a single line (i.e., by a single string). So, it seemed reasonable to me to return a <a class="missing wiki">GapElement?</a> rather than a string.
</p>
<p>
However, this would require many changes in other parts of the code. E.g., I also need a change in
<code>/local/lib/python2.5/site-packages/sage/rings/polynomial/polynomial_ring.py</code>.
This seems not to be part of the mercurial repository, so i have no patch for it.
However, the method <code>_gap_init_</code> of the class <code>PolynomialRing_general</code> should be like that:
</p>
<pre class="wiki"> def _gap_init_(self):
br=self.base_ring()._gap_init_()
if isinstance(br,str):
return 'PolynomialRing(%s, ["%s"])'%(br, self.variable_name())
return br.PolynomialRing('["%s"]'%(self.variable_name()))
</pre><p>
With these patches, the initial problem is almost solved:
</p>
<pre class="wiki">sage: F = CyclotomicField(8)
sage: gap(F)
<algebraic extension over the Rationals of degree 4>
sage: z = F.gen()
sage: a = z+1/z
sage: MS = MatrixSpace(F, 2, 2)
sage: g1 = MS([[1/a,1/a],[1/a,-1/a]])
sage: b = z^2
sage: g2 = MS([[1,0],[0,b]])
sage: g3 = MS([[b,0],[0,1]])
sage: gap(g1)
[ [ -1/2*zeta8^3+1/2*zeta8, -1/2*zeta8^3+1/2*zeta8 ],
[ -1/2*zeta8^3+1/2*zeta8, 1/2*zeta8^3-1/2*zeta8 ] ]
sage: G = MatrixGroup([g1,g2,g3])
sage: G
Matrix group over Cyclotomic Field of order 8 and degree 4 with 3 generators:
[[[-1/2*zeta8^3 + 1/2*zeta8, -1/2*zeta8^3 + 1/2*zeta8], [-1/2*zeta8^3 + 1/2*zeta8, 1/2*zeta8^3 - 1/2*zeta8]], [[1, 0], [0, zeta8^2]], [[zeta8^2, 0], [0, 1]]]
</pre><p>
Why is that only <strong>almost</strong> a solution?
Since the line <code>sage: gap(F)</code> is necessary; otherwise <code>zeta8</code> would not be defined in gap. Hence, it would be needed to change the <code>_gap_init_</code> method of <a class="missing wiki">MatrixSpace?</a>, and so on and so on.
</p>
<p>
<em>Conclusion</em>
</p>
<ul><li>What i do in the patch can't be a definite solution.
</li><li>Is there a way to find a string s so that <code>gap(s)</code> returns a gap version of <code>F</code>, with the variable name <code>zeta8</code>? Having this would probably solve the problem.
</li></ul>
TicketSimonKingSat, 01 Mar 2008 14:34:08 GMTattachment set
https://trac.sagemath.org/ticket/2348
https://trac.sagemath.org/ticket/2348
<ul>
<li><strong>attachment</strong>
set to <em>numberfields_gap.2.patch</em>
</li>
</ul>
<p>
This may be close to a solution
</p>
TicketSimonKingSat, 01 Mar 2008 14:55:49 GMTsummary changed
https://trac.sagemath.org/ticket/2348#comment:9
https://trac.sagemath.org/ticket/2348#comment:9
<ul>
<li><strong>summary</strong>
changed from <em>Teach the gap interface about field extensions</em> to <em>[with patch] Teach the gap interface about field extensions</em>
</li>
</ul>
<p>
The second patch (again relative to rc0-version) may be close to a solution. The following now works:
</p>
<pre class="wiki">sage: F=CyclotomicField(8)
sage: z=F.gen()
sage: a=z+1/z
sage: b=z^2
sage: MS=MatrixSpace(F,2,2)
sage: g1=MS([[1/a,1/a],[1/a,-1/a]])
sage: g2=MS([[1,0],[0,b]])
sage: g3=MS([[b,0],[0,1]])
sage: G = MatrixGroup([g1,g2,g3])
sage: gap(g1)
[ [ -1/2*zeta8^3+1/2*zeta8, -1/2*zeta8^3+1/2*zeta8 ],
[ -1/2*zeta8^3+1/2*zeta8, 1/2*zeta8^3-1/2*zeta8 ] ]
sage: G
Matrix group over Cyclotomic Field of order 8 and degree 4 with 3 generators:
[[[-1/2*zeta8^3 + 1/2*zeta8, -1/2*zeta8^3 + 1/2*zeta8], [-1/2*zeta8^3 + 1/2*zeta8, 1/2*zeta8^3 - 1/2*zeta8]], [[1, 0], [0, zeta8^2]], [[zeta8^2, 0], [0, 1]]]
</pre><p>
The suggested solution works as follows.
</p>
<ul><li>F._gap_init_() returns the <em>name</em> of a gap object corresponding to F. That name is stored in the dictionary of F. If it does not exist in the dictionary, then the gap object is created first; so, that happens only once.
</li><li>If g is an element of F, then <code>g._gap_init_()</code> first checks whether there is already a gap version of <code>g.parent()</code> (which is F). If there isn't, <code>F._gap_init_()</code> is called and creates that object. In either case, <code>g.__repr__()</code> is returned.
</li><li>If M is a matrix with coefficients in F then the existing methods can remain unchanged.
</li></ul><p>
Do you think that approach makes sense? And by the way: I raise a <a class="missing wiki">NotImplementedError?</a> if F.is_absolute()==False, because gap can not deal with non-simple extensions.
</p>
<p>
I still see some problems:
</p>
<ul><li>gap(G) does not work in the above example. So, the <code>_gap_init_</code> method for matrix groups needs being fixed.
</li><li>In my suggested solution, it is not possible to work with non-standard gap interfaces: F._gap_init_() returns a name that is defined in the standard gap interface. Do you have an idea how this can be fixed?
</li></ul>
TicketSimonKingSat, 01 Mar 2008 23:52:33 GMTattachment set
https://trac.sagemath.org/ticket/2348
https://trac.sagemath.org/ticket/2348
<ul>
<li><strong>attachment</strong>
set to <em>final_numberfields_gap.patch</em>
</li>
</ul>
<p>
The patch is relative to sage-2.10.3.rc0 and replaces the previous patches. I think it provides a solution of the problem
</p>
TicketSimonKingSun, 02 Mar 2008 00:06:40 GMTsummary changed
https://trac.sagemath.org/ticket/2348#comment:10
https://trac.sagemath.org/ticket/2348#comment:10
<ul>
<li><strong>summary</strong>
changed from <em>[with patch] Teach the gap interface about field extensions</em> to <em>[with patch, needs review] Teach the gap interface about field extensions</em>
</li>
</ul>
<p>
I think that the last patch provides a solution. Now, simple algebraic extensions of the rationals and matrices over such extensions can by send through the gap interface. By consequence, Matrix Groups over such extensions work.
</p>
<p>
Examples:
</p>
<pre class="wiki">sage: F=CyclotomicField(8)
sage: z=F.gen()
sage: a=z+1/z
sage: MS=MatrixSpace(F,2,2)
sage: g1=MS([[1/a,1/a],[1/a,-1/a]])
sage: b=z^2
sage: g2=MS([[1,0],[0,b]])
sage: g3=MS([[b,0],[0,1]])
sage: gap(g1)*gap(g2)
[ [ (1/2*a-1/2*a^3), (1/2*a+1/2*a^3) ], [ (1/2*a-1/2*a^3), (-1/2*a-1/2*a^3) ] ]
</pre><p>
Remark: So far, the generator of the gap-version of F is alway displayed as 'a'. I did not learn yet how to make it being displayed by gap as 'zeta8', which is how sage displays the generator of F.
</p>
<pre class="wiki">sage: (gap(g1)*gap(g2))^12
[ [ !-1, !0 ], [ !0, !-1 ] ]
</pre><p>
Remark: '!-1' is the integer -1 interpreted in the gap number field.
</p>
<pre class="wiki">sage: G = MatrixGroup([g1,g2,g3])
sage: G
Matrix group over Cyclotomic Field of order 8 and degree 4 with 3 generators:
[[[-1/2*zeta8^3 + 1/2*zeta8, -1/2*zeta8^3 + 1/2*zeta8], [-1/2*zeta8^3 + 1/2*zeta8, 1/2*zeta8^3 - 1/2*zeta8]], [[1, 0], [0, zeta8^2]], [[zeta8^2, 0], [0, 1]]]
sage: G.order()
192
</pre><p>
The last line is based on applying a gap method to G. So, it seems to me that everything works. Making the generator of gap(F) appear as 'a' would probably be a trivial change.
</p>
TicketwdjSun, 02 Mar 2008 11:45:57 GMTsummary changed
https://trac.sagemath.org/ticket/2348#comment:11
https://trac.sagemath.org/ticket/2348#comment:11
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] Teach the gap interface about field extensions</em> to <em>[with patch, positive review] Teach the gap interface about field extensions</em>
</li>
</ul>
<p>
Applies cleanly and fixes the problem. Great job Simon!
Recommend acceptance.
</p>
TicketSimonKingSun, 02 Mar 2008 14:14:03 GMT
https://trac.sagemath.org/ticket/2348#comment:12
https://trac.sagemath.org/ticket/2348#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/2348#comment:11" title="Comment 11">wdj</a>:
</p>
<blockquote class="citation">
<p>
Applies cleanly and fixes the problem. Great job Simon!
Recommend acceptance.
</p>
</blockquote>
<p>
There is one caveat: I had to fix the _gap_init_ method of matrices, since in gap an expression of the form <a class="missing wiki">field elements,...],[...?</a> is not a matrix. That expression becomes a matrix in gap only when multiplied with One(field).
</p>
<p>
By consequence, the doc test of the _gap_init_ method of <a class="missing wiki">MatrixGroup?</a> has to be modified. That modification is part of the patch provided in ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/2367" title="enhancement: [with patch, positive review] Extend invariant_generators to the case ... (closed: invalid)">#2367</a>.
</p>
TicketSimonKingSun, 02 Mar 2008 14:56:25 GMT
https://trac.sagemath.org/ticket/2348#comment:13
https://trac.sagemath.org/ticket/2348#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/2348#comment:11" title="Comment 11">wdj</a>:
</p>
<blockquote class="citation">
<p>
Recommend acceptance.
</p>
</blockquote>
<p>
I still think it may be premature to include the patch. Nathan Dunfield gave me a hint on sage-support <a class="ext-link" href="http://groups.google.com/group/sage-support/browse_thread/thread/ee3c23ea1b86cfe9?hl=en"><span class="icon"></span>http://groups.google.com/group/sage-support/browse_thread/thread/ee3c23ea1b86cfe9?hl=en</a>
</p>
<p>
I think it would be better to change _gap_init_() for number fields according to Nathan's hint. But i think the rest of the patch can stay as it is.
</p>
<p>
I hope tomorrow i will be able to submit a "cleaner" patch.
</p>
TicketSimonKingSun, 02 Mar 2008 17:33:51 GMT
https://trac.sagemath.org/ticket/2348#comment:14
https://trac.sagemath.org/ticket/2348#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/2348#comment:13" title="Comment 13">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I still think it may be premature to include the patch. Nathan Dunfield gave me a hint on sage-support <a class="ext-link" href="http://groups.google.com/group/sage-support/browse_thread/thread/ee3c23ea1b86cfe9?hl=en"><span class="icon"></span>http://groups.google.com/group/sage-support/browse_thread/thread/ee3c23ea1b86cfe9?hl=en</a>
</p>
<p>
I think it would be better to change _gap_init_() for number fields according to Nathan's hint.
</p>
</blockquote>
<p>
No, it doesn't work. If field elements are created using inline functions, they belong to different (but isomorphic) fields in gap and thus can not be added. Note that in the following example, x1 and x2 have the same definition, but can't be added to each other.
</p>
<pre class="wiki">sage: x1=gap('GeneratorsOfField(CallFuncList(function() local x,E; x:=Indeterminate(Rationals,"x"); E:=AlgebraicExtension(Rationals,x^4 + 1); return E; end, []))[1]')
sage: x2=gap('GeneratorsOfField(CallFuncList(function() local x,E; x:=Indeterminate(Rationals,"x"); E:=AlgebraicExtension(Rationals,x^4 + 1); return E; end, []))[1]')
sage: x2+x2
(2*a)
sage: x1+x2
<Traceback>
</pre><p>
So, i will return to my previous approach, but taking more care about the doc tests.
</p>
TicketSimonKingSun, 02 Mar 2008 22:05:35 GMTattachment set
https://trac.sagemath.org/ticket/2348
https://trac.sagemath.org/ticket/2348
<ul>
<li><strong>attachment</strong>
set to <em>fix_doctests_gap_numberfield.patch</em>
</li>
</ul>
<p>
First apply the previous patch, then this patch. It fixes and extends doc tests related with the gap interface of number fields
</p>
TicketSimonKingSun, 02 Mar 2008 22:18:28 GMT
https://trac.sagemath.org/ticket/2348#comment:15
https://trac.sagemath.org/ticket/2348#comment:15
<p>
The patch fix_doctests_gap_numberfield.patch should be applied after final_numberfields_gap.patch.
</p>
<p>
The new patch fixes some problems with doc tests. Also, it adds more doc tests. Moreover, now the generator of a number field gets the same name in gap and in sage, which may be convenient for the users.
</p>
<p>
The following doctests related with and extended by the patch pass:
matrix_group.py,
number_field.py,
number_field_element.pyx,
matrix1.pyx
</p>
<p>
@wdj: Could you please see if your positive review still holds when adding the new patch?
</p>
TicketSimonKingSun, 02 Mar 2008 22:28:56 GMTmilestone changed
https://trac.sagemath.org/ticket/2348#comment:16
https://trac.sagemath.org/ticket/2348#comment:16
<ul>
<li><strong>milestone</strong>
changed from <em>sage-3.0</em> to <em>sage-2.10.3</em>
</li>
</ul>
TicketwdjSun, 02 Mar 2008 23:37:23 GMT
https://trac.sagemath.org/ticket/2348#comment:17
https://trac.sagemath.org/ticket/2348#comment:17
<p>
This applied cleanly against 2.10.3.rc0 and passed sage -testall.
Positive review (again).
</p>
TicketSimonKingWed, 05 Mar 2008 10:47:22 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/2348#comment:18
https://trac.sagemath.org/ticket/2348#comment:18
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>invalid</em>
</li>
</ul>
<p>
This ticket, together with <a class="closed ticket" href="https://trac.sagemath.org/ticket/2367" title="enhancement: [with patch, positive review] Extend invariant_generators to the case ... (closed: invalid)">#2367</a>, is now ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/2395" title="enhancement: [with patch, positive review] New features for number fields (gap ... (closed: fixed)">#2395</a>. First reason is that we got a new sage pre-release.
</p>
<p>
Second reason is that my suggestions in this ticket for fixing the gap interface ought to be changed: By hints of Nathan Dunfield, i learned that _gap_init_ is in fact called only once, and the objects in the gap interface are cached. Hence, using gap inline functions work. This different approach is in ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/2395" title="enhancement: [with patch, positive review] New features for number fields (gap ... (closed: fixed)">#2395</a>.
</p>
<p>
Third reason is that <a class="closed ticket" href="https://trac.sagemath.org/ticket/2367" title="enhancement: [with patch, positive review] Extend invariant_generators to the case ... (closed: invalid)">#2367</a> and this ticket belong closely together, hence, they should be worked on in one ticket.
</p>
Ticket