On Thu, Mar 15, 2012 at 1:29 PM, Mike OS <[email protected]> wrote:
> 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

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 <[email protected]>
     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

-- 
You received this message because you are subscribed to the Google Groups 
"sage-edu" 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-edu?hl=en.

Reply via email to