#5513: [with patch, positive review subject to minor alterations] Enhanced 
support
for number field unit groups
---------------------------+------------------------------------------------
 Reporter:  cremona        |       Owner:  was                    
     Type:  enhancement    |      Status:  new                    
 Priority:  major          |   Milestone:  sage-3.4.1             
Component:  number theory  |    Keywords:  number field unit group
---------------------------+------------------------------------------------

Comment(by fwclarke):

 This seems to work fine, with some exceptions for relative number fields.
 For example, with my current favourite extension,
 {{{
 PQ.<X> = QQ[]
 F.<a, b> = NumberField([X^2 - 2, X^2 - 3])
 PF.<Y> = F[]
 K.<c> = F.extension(Y^2 - (1 + a)*(a + b)*a*b)
 K.unit_group()
 }}}
 fails.

 But the cause lies beyond this patch.  There are many problems with the
 existing code for relative number fields, some addressed in my patch in
 #5508, others to follow soon in a revised patch.

 In this case the culprit is the use of `pari_bnf` in the line `pK =
 K.pari_bnf(proof)`
 at line 145 of `unit_group.py`.  For in `pari_bnf`,
 the lines
 {{{
 if self.polynomial().denominator() != 1:
     raise TypeError, "Unable to coerce number field defined by ..."
 }}}
 exclude extensions like `K`.  The solution is, of course, to use
 `self.absolute_polynomial()`; this change could be included
 in the patch.

 So my review is positive, subject to some small changes listed below,
 with the expectation that forthcoming changes to other functions will make
 the code work for all (sufficiently small) relative number fields.

 == Minor points ==

    * Three times:
       "but we do use if it is already known"
       should surely be
       "but we do use it if it is already known"

    * In `zeta`,
       {{{
               z = self.primitive_root_of_unity() ** (N//n)`
       }}}
      would be more consistently written as
      {{{
               z = K.primitive_root_of_unity() ** (N//n)`
      }}}


    * In `roots_of_unity`
       {{{
               n = z.multiplicative_order()
       }}}
      would surely be better as
      {{{
               n = self.zeta_order()
      }}}

 == Further remark ==

 While we're working on units, one thing that should be fixed is the
 following:
 {{{
 sage: K.<b> = NumberField([x^2 - 3])
 sage: K.zeta(5)
 Traceback (most recent call last)
 ...
 ArithmeticError: There are no roots of unity of order 5 in this unit group
 }}}
 compared to
 {{{
 sage: C.<z> = CyclotomicField(20)
 sage: C.zeta(7)
 Traceback (most recent call last)
 ...
 ValueError: n (=7) does not divide order of generator
 }}}
 There seems to be no reason why the error type should be different, and I
 found an example where a function failed for cyclotomic fields because of
 this disparity.

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5513#comment:2>
Sage <http://sagemath.org/>
Sage - Open Source Mathematical Software: Building the Car Instead of 
Reinventing the Wheel

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/sage-trac?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to