Ticket #1849 (new defect)

Opened 1 year ago

Last modified 7 months ago

[with patch (part 1 of 2); not ready for review] rewrite abelian groups

Reported by: was Assigned to: joyner
Priority: major Milestone: sage-3.4
Component: group_theory Keywords: editor_roed
Cc:

Description 


On Jan 19, 2008 8:00 AM, John Cremona <john.cremona@gmail.com> wrote:
> 
> Either this is a bug or I have been teaching students the wrong thing for years:
> 
> (in 2.9.3)
> 
> sage: A=AbelianGroup([7,8])
> 
> sage: A.invariants()
>  [7,8]
> 
> sage: A.elementary_divisors()
>  [7,8]
> 
> There are two sets of invariants for a finite abelian group.  The
> "elementary divisors" should be a list of integers n_1,n_2,...,n_k
> with each dividing the next such that A is the direct sum of cyclic
> groups of order n_i.  In this case A is cyclic so the list should be
> just [56].  The other set is a list of prime powers and in this case
> is indeed [7,8] though we could argue about the ordering.
> 
> Judging by the docstrings, the implemeter of these things in Sage does
> not agree with me, since the code seems to assume that the
> "invariants" are what I am calling the elem. divisors, and the elem.
> divisors are the prime-power factors of the invariants, sorted.  In
> which case the invariants is listed above are wrong!
> 
> The Sage version of this is all explained in the documentation in file
> sage/groups/abelian_gps/abelian_groups.py, attributed to David Joyner
> .  Please will all the algebraists out there tell me whether tey agree
> with me or him!  I'm not in a position to consult textbooks right now.
>  The docs there refer to Henri Cohen's books so it is possible that
> there is a French/English split here BUT in any case the group I
> defined about is *cyclic* so one of the two outputs is wrong, even if
> we disagree which one!

The problem is that when David Joyner implemented this, doing 

sage: A=AbelianGroup([7,8])

would immediately change the invariants to be the elementary divisors. 
I.e., it was *impossible* 
to make an abelian group with specified generator orders -- the would
also change to be elementary divisors.   This was so
hugely painful to use that I fixed AbelianGroup so that one could create
a group with arbitrary generator orders.   Unfortunately, I evidently
didn't correctly update the elementary_divisors function.

William

Attachments

trac-1849-abelian-group-rewrite-part1.patch (39.1 kB) - added by was on 01/27/2008 10:20:05 PM.
this is part 1 -- doctests outside abelian groups directory will be broken.
abelian_groups_rewrite_part_2.patch (18.8 kB) - added by was on 02/01/2008 03:59:41 PM.
this is part 2. This is also not ready.

Change History

01/27/2008 10:19:04 PM changed by was

  • summary changed from bug in elementary_divisors for abelian groups to [with patch (part 1 of 2); not ready for review] rewrite abelian groups.

01/27/2008 10:20:05 PM changed by was

  • attachment trac-1849-abelian-group-rewrite-part1.patch added.

this is part 1 -- doctests outside abelian groups directory will be broken.

02/01/2008 03:59:41 PM changed by was

  • attachment abelian_groups_rewrite_part_2.patch added.

this is part 2. This is also not ready.

02/06/2008 03:02:06 PM changed by was

I just found this terrifying line in abelian_group.py: 

Hgensf = [x for x in Hgens if len(set(Ggens0).intersection(set(list(str(x)))))==0]

OUCH! This will completely break if one makes, e.g., abelian groups with generators that have more than one letter. For example, in current Sage 

sage: G.<a,aa,aaa> = AbelianGroup(3,[5,0,0])
sage: G.subgroup([a])

Multiplicative Abelian Group isomorphic to C5, which is the subgroup of
Multiplicative Abelian Group isomorphic to C5 x Z x Z
generated by [a]
sage: G.subgroup([aa])
---------------------------------------------------------------------------
<type 'exceptions.RuntimeError'>          Traceback (most recent call last)

/Users/was/<ipython console> in <module>()

/Users/was/sage2/local/lib/python2.5/site-packages/sage/groups/abelian_gps/abelian_group.py in subgroup(self, gensH, names)
    701             if not(gensH[i] in G):
    702                 raise TypeError, "Subgroup generators must belong to the given group."
--> 703         return AbelianGroup_subgroup(self, gensH, names)
    704 
    705     def __cmp__(self, right):

/Users/was/sage2/local/lib/python2.5/site-packages/sage/groups/abelian_gps/abelian_group.py in __init__(self, ambient, gens, names)
    927                gap.eval(cmd)
    928         s2 = "gensH:=%s"%Hgensf #### remove from this the ones <--> 0 invar
--> 929         gap.eval(s2)
    930         s3 = 'H:=Subgroup(G,gensH)'
    931         gap.eval(s3)

/Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/gap.py in eval(self, x, newlines, strip)
    307         if len(x) == 0 or x[len(x) - 1] != ';':
    308             x += ';'
--> 309         s = Expect.eval(self, x)
    310         if newlines:
    311             return s

/Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/expect.py in eval(self, code, strip, **kwds)
    705         try:
    706             with gc_disabled():
--> 707                 return '\n'.join([self._eval_line(L, **kwds) for L in code.split('\n') if L != ''])
    708         except KeyboardInterrupt:
    709             # DO NOT CATCH KeyboardInterrupt, as it is being caught

/Users/was/sage2/local/lib/python2.5/site-packages/sage/interfaces/gap.py in _eval_line(self, line, allow_use_file, wait_for_prompt)
    508                         return ''
    509                 else:
--> 510                     raise RuntimeError, message
    511 
    512         except KeyboardInterrupt:

<type 'exceptions.RuntimeError'>: Gap produced error output
Variable: 'aa' must have a value


   executing gensH:=[aa];

04/26/2008 12:08:28 AM changed by mabshoff

#517 is in the same ball park.

Cheers,

Michael

05/07/2008 03:25:26 PM changed by was

See also #3127 and #2272.

06/19/2008 09:38:46 PM changed by craigcitro

  • keywords set to editor_roed.