Sage: Ticket #5241: [with patch, positive review] Matrix Group sometimes assumes base ring is a field
https://trac.sagemath.org/ticket/5241
<p>
From sage-support:
</p>
<pre class="wiki">sage: M1 = matrix(ZZ,2,[[-1,0],[0,1]])
sage: M2 = matrix(ZZ,2,[[1,0],[0,-1]])
sage: M3 = matrix(ZZ,2,[[-1,0],[0,-1]])
sage: MG = MatrixGroup([M1, M2, M3])
sage: MG.order()
4
sage: MG.list()
Traceback (click to the left for traceback)
...
AttributeError: 'sage.rings.integer_ring.IntegerRing_class' object
has
no attribute 'prime_subfield'
</pre><p>
The offending code is in groups/matrix_gps/matrix_group.py where the problem is in the list method of <a class="missing wiki">MatrixGroup?</a>.
</p>
<pre class="wiki">429 F = self.field_of_definition()
430 n = F.degree()
431 p = F.characteristic()
432 a = F.prime_subfield().multiplicative_generator()
433 b = F.multiplicative_generator()
</pre><p>
In the class definition of <a class="missing wiki">MatrixGroup?</a>, any base ring is allowed. But at least this particular method (others?) assume that the base ring is in fact in a field.
</p>
<p>
Since a and b above are definitely used later in list(), as this all calls GAP, someone with a good knowledge of GAP should address this - and look for other places it's assumed that the base ring is a field and at least catch that exception with a better error message.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/5241
Trac 1.1.6wdjTue, 17 Feb 2009 02:53:46 GMTattachment set
https://trac.sagemath.org/ticket/5241
https://trac.sagemath.org/ticket/5241
<ul>
<li><strong>attachment</strong>
set to <em>trac_5241-matrix-gp-list.patch</em>
</li>
</ul>
<p>
to be applied to sage-3.3.alpha6
</p>
TicketwdjTue, 17 Feb 2009 02:55:24 GMTsummary changed
https://trac.sagemath.org/ticket/5241#comment:1
https://trac.sagemath.org/ticket/5241#comment:1
<ul>
<li><strong>summary</strong>
changed from <em>Matrix Group sometimes assumes base ring is a field</em> to <em>[with patch, needs review] Matrix Group sometimes assumes base ring is a field</em>
</li>
</ul>
<p>
This passes sage -t but sage -testall has lots of (unrelated I think) failures. It simply fixes the bug reported and adds the required docstring examples.
</p>
TicketkcrismanFri, 06 Mar 2009 15:45:00 GMT
https://trac.sagemath.org/ticket/5241#comment:2
https://trac.sagemath.org/ticket/5241#comment:2
<p>
Will this need rebase against 3.4?
</p>
TicketkcrismanMon, 16 Mar 2009 15:25:21 GMTsummary changed; cc set
https://trac.sagemath.org/ticket/5241#comment:3
https://trac.sagemath.org/ticket/5241#comment:3
<ul>
<li><strong>cc</strong>
<em>joyner</em> added
</li>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] Matrix Group sometimes assumes base ring is a field</em> to <em>[with patch, needs work] Matrix Group sometimes assumes base ring is a field</em>
</li>
</ul>
<p>
Against 3.4:
</p>
<p>
patching file sage/groups/matrix_gps/matrix_group.py
Hunk <a class="closed ticket" href="https://trac.sagemath.org/ticket/1" title="defect: SAGE Notebook leaves dead processes on OS X (closed: fixed)">#1</a> FAILED at 410
1 out of 3 hunks FAILED -- saving rejects to file sage/groups/matrix_gps/matrix_group.py.rej
abort: patch failed to apply
</p>
<p>
The hunk that didn't apply was the documentation hunk, as it turns out, unsurprisingly.
</p>
<p>
So it does need a rebase. Sorry it took a while to be able to check this; it's sometimes complicated for me to do these things in a timely fashion, particularly with the 3.4 upgrade this was the case.
</p>
TicketmabshoffMon, 16 Mar 2009 16:00:01 GMTsummary changed
https://trac.sagemath.org/ticket/5241#comment:4
https://trac.sagemath.org/ticket/5241#comment:4
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs work] Matrix Group sometimes assumes base ring is a field</em> to <em>[with patch, needs rebase] Matrix Group sometimes assumes base ring is a field</em>
</li>
</ul>
TicketAlexGhitzaSun, 31 May 2009 13:19:41 GMTsummary changed
https://trac.sagemath.org/ticket/5241#comment:5
https://trac.sagemath.org/ticket/5241#comment:5
<ul>
<li><strong>summary</strong>
changed from <em>[with patch, needs rebase] Matrix Group sometimes assumes base ring is a field</em> to <em>[with patch, positive review] Matrix Group sometimes assumes base ring is a field</em>
</li>
</ul>
<p>
I have rebased the patch against 4.0 and tested it. Looks good to me.
</p>
<p>
Apply only <code>trac_5241-rebased.patch</code>.
</p>
TicketAlexGhitzaSun, 31 May 2009 13:20:11 GMTattachment set
https://trac.sagemath.org/ticket/5241
https://trac.sagemath.org/ticket/5241
<ul>
<li><strong>attachment</strong>
set to <em>trac_5241-rebased.patch</em>
</li>
</ul>
<p>
rebased against 4.0
</p>
TicketmhansenMon, 01 Jun 2009 05:36:29 GMTstatus changed; resolution set
https://trac.sagemath.org/ticket/5241#comment:6
https://trac.sagemath.org/ticket/5241#comment:6
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Merged in 4.0.1.alpha0.
</p>
TicketmhansenSat, 06 Jun 2009 21:51:01 GMTreviewer, merged, author set
https://trac.sagemath.org/ticket/5241#comment:7
https://trac.sagemath.org/ticket/5241#comment:7
<ul>
<li><strong>reviewer</strong>
set to <em>Alex Ghitza</em>
</li>
<li><strong>merged</strong>
set to <em>4.0.1.alpha0</em>
</li>
<li><strong>author</strong>
set to <em>David Joyner</em>
</li>
</ul>
Ticket