Changes between Initial Version and Version 1 of Ticket #10625


Ignore:
Timestamp:
01/16/11 08:28:41 (2 years ago)
Author:
davidloeffler
Comment:

I don't mean to seem hostile, but: I wrote the generic Smith and Hermite form code, and I used no assumptions on the methods of the ring elements other than those mandated by the docstrings of the top-level sage.structure.element.RingElement class. The problem is that the Integer class doesn't play by the rules (because it's so routine to confuse an ideal of ZZ with its non-negative generator).

So the solution is not to change the Smith form code, but to add a couple of lines to Integer.inverse_mod to make it consistent with the general ring element template.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #10625

    • Property Summary changed from Generic smith form of a matrix fails on integer matrix to inverse_mod for integer ring won't take an ideal as argument
  • Ticket #10625 – Description

    initial v1  
    1 The generic Smith Form routine fails for integer matrices, making me suspicious it needs some attention.  The specialized version for integer matrices seems fine, so this is not a problem per se, just evidence something is amiss.  Here's the evidence, obtained by shipping in a sparse matrix - this should soon be fixed by sending sparse matrices to the working dense version.  So if you test this ''use something like 4.6.1 or earlier''. 
     1The top level {{{inverse_mod}}} method for ring elements (in {{{sage.structure.element.RingElement}}}) mandates that {{{inverse_mod}}} should always take an ideal of the ring as its argument. The integer ring doesn't implement this correctly: 
     2 
     3{{{ 
     4sage: ZZ(2).inverse_mod(ZZ.ideal(3)) 
     5--------------------------------------------------------------------------- 
     6TypeError                                 Traceback (most recent call last) 
     7 
     8/storage/masiao/sage-4.6.2.alpha0/devel/sage-reviewing/sage/rings/<ipython console> in <module>() 
     9 
     10/usr/local/sage/sage-4.6.1/local/lib/python2.6/site-packages/sage/rings/integer.so in sage.rings.integer.Integer.inverse_mod (sage/rings/integer.c:28740)() 
     11   5161         cdef Integer ans = <Integer>PY_NEW(Integer) 
     12   5162         if mpz_cmp_ui(m.value, 1) == 0: 
     13-> 5163             return zero 
     14   5164         sig_on() 
     15   5165         r = mpz_invert(ans.value, self.value, m.value) 
     16 
     17/usr/local/sage/sage-4.6.1/local/lib/python2.6/site-packages/sage/rings/integer.so in sage.rings.integer.as_Integer (sage/rings/integer.c:6052)() 
     18    365         return <Integer>x 
     19    366     else: 
     20--> 367         return Integer(x) 
     21    368  
     22    369 cdef class IntegerWrapper(Integer): 
     23 
     24/usr/local/sage/sage-4.6.1/local/lib/python2.6/site-packages/sage/rings/integer.so in sage.rings.integer.Integer.__init__ (sage/rings/integer.c:7312)() 
     25    678                         pass 
     26    679                          
     27--> 680                 raise TypeError, "unable to coerce %s to an integer" % type(x) 
     28    681  
     29    682     def __reduce__(self): 
     30 
     31TypeError: unable to coerce <class 'sage.rings.ideal.Ideal_pid'> to an integer 
     32}}} 
     33 
     34This causes issues with the generic Smith form implementation applied to integer matrices. Here's the evidence, obtained by shipping in a sparse matrix - this should soon be fixed by sending sparse matrices to the working dense version.  So if you test this ''use something like 4.6.1 or earlier''. 
    235 
    336{{{ 
     
    2558TypeError: unable to coerce <class 'sage.rings.ideal.Ideal_pid'> to an integer 
    2659}}} 
    27  
    28 On the input above, I think the failure is in `sage.matrix.matrix2._generic_clear_column` at the line: 
    29  
    30 {{{ 
    31 c = R(a[0,0] / B).inverse_mod(R.ideal(a[k,0] / B)) 
    32 }}} 
    33  
    34 which would be  
    35  
    36 {{{ 
    37 ZZ(2/1).inverse_mod(ZZ.ideal(3/1)) 
    38 }}} 
    39  
    40 which by itself fails in a similar fashion. 
    41  
    42 Can you place an ideal inside `Integer.inverse_mod`?