Ticket #6449 (needs_work defect)

Opened 8 months ago

Last modified 2 months ago

[with patches, not ready for review] Additive abelian groups

Reported by: davidloeffler Owned by: joyner
Priority: major Milestone: sage-feature
Component: group_theory Keywords: abelian group
Cc: jhpalmieri Author(s): David Loeffler
Report Upstream: N/A Reviewer(s):
Merged in: Work issues:

Description

This is the results of the Abelian groups project at SD16. This is not really finished, but I am going to be pretty busy for the next few weeks so I am posting it here anyway in the hope that somebody else will chip in and finish it off.

This relies on all of the patches at #5882.

The new functionality:

- A new class for additive abelian groups. This is very similar to the finitely presented modules class, but with a slightly different print representation. This is mostly finished (although the string representation of morphisms needs a little work).

- A class for "additive abelian groups embedded in an arbitrary parent". Think rational points on an elliptic curve, for instance, which should be able to derive from this class and transparently inherit code for things like morphisms and subgroups. This is a lot less complete.

Attachments

trac_6449-1-abgps.patch Download (15.4 KB) - added by davidloeffler 8 months ago.
trac_6449-2-homology.patch Download (2.8 KB) - added by davidloeffler 8 months ago.
trac_6449-3-elliptic.patch Download (29.1 KB) - added by davidloeffler 8 months ago.

Change History

Changed 8 months ago by jhpalmieri

  • cc jhpalmieri added

Changed 8 months ago by davidloeffler

Changed 8 months ago by davidloeffler

Changed 8 months ago by davidloeffler

Changed 8 months ago by davidloeffler

  • summary changed from Additive abelian groups to [with patches, not ready for review] Additive abelian groups

Here are three patches: the first is the new classes, and the second make alterations to the two places where abelian groups are used "in an additive way" in the Sage library to convert them over (which I did chiefly in order to get a feel for how well it would owrk in practice).

Changed 8 months ago by jlefebvre

I raised this issue in  http://trac.sagemath.org/sage_trac/ticket/6291 , that abelian group has different interface that other groups. In particular, they have no identity function, as opposed to all other groups I've tried in Sage. It's a minor thing, but it makes it annoying when you have code that works with arbitrary group. I've looked at the patch, and it doesn't seem to implement it. I figured I should just complain about it now instead of later :)

Changed 2 months ago by cremona

  • keywords abelian group added
  • upstream set to N/A
  • author set to David Loeffler

I'm hoping that we can sort this out and get it finished!

Comments on the patches:

  1. Lines 47-53 of patch 1. Not sure I understand the issue here. Isn't each coordinate just reduced modulo the appropriate integer (if positive)?
  2. Some functionality (e.g. annihilator()) is presumably inherited from one of the base classes. I think it would help if this was written down explicitly in comments, especially as there is more than one base class.
  3. As well as an is_multiplicative() returning False, should tere not be an is_additive() returning True? Or does that exist already from a base class?
  4. You mention a black-box discrete log would be desirable. We have that in sage/groups/generic.py -- does that do what is needed here? Or is the problem that that only deals with the cyclic case?
  5. Patch 2 looks quite simple, I have not looked at it in detail.
  6. Patch 3 is mainly changes on doctest output, but also shows how some code can be simplified using the new framework, which is good.

Now I'll actually try applying the patches to 4.3 and see how it looks.

Note: See TracTickets for help on using tickets.