Abelian Groups: comments and suggestions

82 views
Skip to first unread message

Mike OS

unread,
Mar 15, 2012, 4:29:44 PM3/15/12
to sage-edu
I am using sage in a graduate algebra class, and would like to expand
to
other courses. I've found a few things that are incomplete,
inconsistent or
that would be confusing to students and realized it might be good to
organize and
catalogue them for sage developers, to give a user/teacher's
perspective.
This critique is offered with the utmost respect and gratitude for
what you have all created!

This post concerns abelian groups.
It seems like abelian groups is a backwater in sage;
I suppose they are not terribly interesting for research!
They can be valuable pedagogically. As such, it is important
that the notation be consistent and simple.
I like AdditiveAbelianGroups more than
AbelianGroups for the purposes of teaching,
since the GAP notation of the latter is more
intimidating for students.

********
AddAbGrp: (short for AdditiveAbelianGroup) lacks some functionality.
(1) elementary divisors()
(2) subgroups()
(3) subgroup
(3) A.addition_table() hangs my machine
(4) A.direct_sum (or direct_product, cartesian_product returns a set
not an AddAbGrp)

(5) In
A = AdditiveAbelianGroup([8, 5])
Additive abelian group isomorphic to Z/40
I would rather be told
Additive abelian group Z/8 + Z/5
Since that is how I created it and that is how elements will be
written.
The following is not good in my opinion.
A = AdditiveAbelianGroup([5, 5,8])
A([1,1,3])
error
A([1,2])
(1, 1, 2)

********
AbelianGroups: has some bugs.
(1) is_cyclic() is sometimes incorrect
B= AbelianGroup([3,4,5])
sage: B.is_cyclic()
False
sage: B= AbelianGroup([3,4])
sage: B.is_cyclic()
True
(2) B.quotient() is not implemented.

(3) B.elementary divisors returns the invariant factors.
B.invariants returns the integers used to define the group,
not the invariant factors.
sage: B= AbelianGroup([10,12,25])
sage: B.invariants()
[10, 12, 25]
sage: B.elementary_divisors()
[10, 300]

*********
Wish list: For AddAbGrp

(0) Given a list of elements do the elements generate the group
(is_generating() )
(1) Create a homomorphism A -> B or be told my hom is ill-defined.
(2) Create a quotient and hom to the quotient.
(3) Compute direct product
(4) Create an isomorphism to the invariant factor group and the
elementary divisor group.
(5) Compute the automorphism group of an abelian group AddAbGrp ->
PermutationGrp
A.permutation_group() creates the image but I dont think it gives
the map.
(6) Coerce an abelian group in some other category into AddAbGrp (so
that I can get its invariants).
(a) Given an abelian permutation group G, or matrix group, create an
isomorphism to an abelian group.
(b) Given an the unit group of a commutative ring (say
Zn.unit_group()) create an iso to an AbelianGroup.

William Stein

unread,
Mar 15, 2012, 4:50:08 PM3/15/12
to sage...@googlegroups.com, Cremona John, David Joyner

It depends on A. It works fine for me with input "A =
AdditiveAbelianGroup([8, 5])".

> (4) A.direct_sum (or direct_product, cartesian_product returns a set
> not an AddAbGrp)
>
> (5) In
>   A = AdditiveAbelianGroup([8, 5])
>   Additive abelian group isomorphic to Z/40
> I would rather be told
>   Additive abelian group Z/8 + Z/5
> Since that is how I created it and that is how elements will be
> written.

Rather than what is done (or what you suggest), maybe we should do
what Magma does:

MAGMA: AbelianGroup([8, 5]);
Abelian Group isomorphic to Z/40
Defined on 2 generators
Relations:
8*$.1 = 0
5*$.2 = 0


> The following is not good in my opinion.
>   A = AdditiveAbelianGroup([5, 5,8])
>   A([1,1,3])
>      error
>   A([1,2])
>      (1, 1, 2)

It is also inconsistent with Magma:

MAGMA: A := AbelianGroup([5,5,8]);
MAGMA: A;
Abelian Group isomorphic to Z/5 + Z/40
Defined on 3 generators
Relations:
5*A.1 = 0
5*A.2 = 0
8*A.3 = 0
MAGMA: A![1,2];

^
Runtime error in '!': Sequence argument length (2) should be 3 to be coerced
into a matrix or vector

MAGMA: A![1,2,3];
A.1 + 2*A.2 + 3*A.3


>
> ********
> AbelianGroups: has some bugs.
> (1) is_cyclic() is sometimes incorrect
> B= AbelianGroup([3,4,5])
> sage: B.is_cyclic()
> False

A fun thing about Sage is not only can you find what causes a bug, you
can see how introduced it, when, and who signed off on it, and what
they said.

The above bug is because of:

sage: B = AbelianGroup([3,4,5])
sage: B.elementary_divisors()
[1, 60]

The code seems to assume that 1 can occur at most once an elementary
divisor (yikes):

if len(eldivs)==1 or not(1 in eldivs):
return eldivs
else:
# eldivs is [1,1,60] at this point
eldivs.remove(1)
return eldivs

Via "hg blame" we find that this is from this changeset:

changeset: 11329:982567bcab23
user: David Joyner <wdjo...@gmail.com>
date: Fri Dec 12 18:46:23 2008 -0500
summary: Edits following suggestions of referee - wdj

That changeset was part of http://trac.sagemath.org/sage_trac/ticket/3749
and the reviewer said "I have been avoiding reviewing this for ages
since I hate the whole abelian groups code so much that every time I
look at it I want to rewrite it from scratch" before signing off
reluctantly to this code.

(I think David Roe or maybe David Loefler tried to rewrite abelian
groups at a Sage Days in Barcelona and failed. I had joked at the
time that this was the 6th attempt.)

> sage: B= AbelianGroup([3,4])
> sage: B.is_cyclic()
> True
> (2) B.quotient() is not implemented.
>
> (3) B.elementary divisors returns the invariant factors.
> B.invariants returns the integers used to define the group,
> not the invariant factors.
> sage: B= AbelianGroup([10,12,25])
> sage: B.invariants()
> [10, 12, 25]
> sage: B.elementary_divisors()
> [10, 300]

That's pretty much "by definition". It's stupid though. Again, Magma
does the right thing:

MAGMA: A := AbelianGroup([5,5,8]);
MAGMA: Invariants(A);
[ 5, 40 ]

>
> *********
> Wish list: For AddAbGrp
>
> (0) Given a list of elements do the elements generate the group
> (is_generating() )
> (1) Create a homomorphism A -> B or be told my hom is ill-defined.
> (2) Create a quotient and hom to the quotient.
> (3) Compute direct product
> (4) Create an isomorphism to the invariant factor group and the
> elementary divisor group.
> (5) Compute the automorphism group of an abelian group AddAbGrp ->
> PermutationGrp
>    A.permutation_group() creates the image but I dont think it gives
> the map.
> (6) Coerce an abelian group in some other category into AddAbGrp (so
> that I can get its invariants).
>  (a) Given an abelian permutation group G, or matrix group, create an
> isomorphism to  an abelian group.
>  (b) Given an the unit group of a commutative ring (say
> Zn.unit_group()) create an iso to an AbelianGroup.

It would be good if somebody rewrote abelian groups from scratch
taking into account your comments above. Personally, I would probably
make the user interface be similar to Magma's abelian groups, which is
pretty well thought out, and will make it easier for people (like me)
to use both Sage and Magma:

http://magma.maths.usyd.edu.au/magma/handbook/abelian_groups

David Joyner who wrote the abelian groups code was much more of a GAP
user (he didn't use Magma much).

-- William

William Stein

unread,
Mar 15, 2012, 4:52:57 PM3/15/12
to sage...@googlegroups.com, Cremona John, David Joyner
On Thu, Mar 15, 2012 at 1:50 PM, William Stein <wst...@gmail.com> wrote:
> On Thu, Mar 15, 2012 at 1:29 PM, Mike OS <mosu...@math.sdsu.edu>
>> ********
>> AbelianGroups: has some bugs.
>> (1) is_cyclic() is sometimes incorrect
>> B= AbelianGroup([3,4,5])
>> sage: B.is_cyclic()
>> False

It occurs to me that this is easy for me to fix having pointed out the
issue, and should be a blocker ticket for sage-5.0. Here is the
ticket. I'll post a patch in a moment:

http://trac.sagemath.org/sage_trac/ticket/12675

--
William Stein
Professor of Mathematics
University of Washington
http://wstein.org

Mike OS

unread,
Mar 16, 2012, 6:16:33 PM3/16/12
to sage-edu
I would certainly like sage to follow Magma's style.
As you say it is really well thought at out, and clear.
Thanks for doing the little fix.

My problem with addition table seems to have evaporated.


Rob Beezer

unread,
May 3, 2012, 4:52:15 PM5/3/12
to sage...@googlegroups.com, Cremona John, David Joyner


On Thursday, March 15, 2012 1:50:08 PM UTC-7, William Stein wrote:

It would be good if somebody rewrote abelian groups from scratch
taking into account your comments above.  Personally, I would probably
make the user interface be similar to Magma's abelian groups, which is
pretty well thought out, and will make it easier for people (like me)
to use both Sage and Magma:

(Been away for a while and missed this thread.)

Agreed.  I might be the 7th attempt.  I started this once, and then when I came back to it a year later, the category code had changed so much that it needed a severe rewrite (which I may have lost).  But I have support this summer for exactly this task and a good idea of how to attack it.

Mike O'S - I'll take your comments into account and would love further feedback.  First attempt is at:

http://trac.sagemath.org/sage_trac/ticket/9773

General strategy:  extend a good idea of Cremona and others (iirc) to build on William's code for finitely generated modules over PID's.  A lot of things (like forming a subgroup, nee submodule) then come for free.

I wanted to build one abstract class, then extend it into additive and multiplicative flavors.  These would then be suitable for building generic cyclic groups, the group of units mod n, the multiplicative subgroup of a finite field, etc, etc.

Rob

David Joyner

unread,
May 4, 2012, 5:43:22 AM5/4/12
to SAGE edu
Forwarded to sage-edu (as requested by John Cremona).


---------- Forwarded message ----------
From: John Cremona <john.c...@gmail.com>
Date: Thu, May 3, 2012 at 5:47 PM
Subject: Re: [sage-edu] Abelian Groups: comments and suggestions
To: Rob Beezer <goo...@beezer.cotse.net>
Cc: sage...@googlegroups.com, David Joyner <wdjo...@gmail.com>


Don't forget that we do have things like

sage: E = EllipticCurve('11a1')
sage: T = E.torsion_subgroup()
sage: T.element_class
<class 'sage.groups.additive_abelian.additive_abelian_wrapper.EllipticCurveTorsionSubgroup_with_category.element_class'>
sage: T
Torsion Subgroup isomorphic to Z/5 associated to the Elliptic Curve
defined by y^2 + y = x^3 - x^2 - 10*x - 20 over Rational Field
sage: O = T(0)
sage: type(O)
<class 'sage.groups.additive_abelian.additive_abelian_wrapper.EllipticCurveTorsionSubgroup_with_category.element_class'>
sage:

using the abelian group wrapper for the PID modules code.  It's
already used in quite a few places.

John
Reply all
Reply to author
Forward
0 new messages