Opened 8 years ago
Closed 8 years ago
#14791 closed enhancement (fixed)
Named finitely presented groups
Reported by:  dshurbert  Owned by:  joyner 

Priority:  minor  Milestone:  sage5.13 
Component:  group theory  Keywords:  group presentations, free groups 
Cc:  rbeezer, vbraun, mmarco  Merged in:  sage5.13.beta0 
Authors:  Davis Shurbert  Reviewers:  Volker Braun, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
Using the existing groups catalog framework, users are given the capability to create "named" groups as finite presentations. The goal of this addition to Sage is to make the concepts of free groups and group presentations more accessible to the typical undergrad, allowing them to work with standard groups introduced in an introductory level abstract algebra course.
sage: groups.presentation.Cyclic(17) Finitely presented group < a  a^17 > sage: groups.presentation.Dihedral(8) Finitely presented group < a, b  a^8, b^2, a*b*a*b > sage: groups.presentation.DiCyclic(5) Finitely presented group < a, b  a^10, b^2*a^5, b^1*a*b*a > sage: groups.presentation.KleinFour() Finitely presented group < a, b  a^2, b^2, a^1*b^1*a*b >
More groups are to be implemented soon. Feedback/suggestions on implementation details are highly encouraged.
Apply
Attachments (3)
Change History (26)
Changed 8 years ago by
comment:1 Changed 8 years ago by
 Description modified (diff)
comment:2 Changed 8 years ago by
 Status changed from new to needs_info
comment:3 Changed 8 years ago by
 Description modified (diff)
comment:4 Changed 8 years ago by
 Status changed from needs_info to needs_review
comment:5 Changed 8 years ago by
The printing of free group elements changed in the latest GAP update, this leads to doctest failures of the kind
Failed example: D = groups.presentation.Dihedral(7); D Expected: Finitely presented group < a, b  a^7, b^2, a*b*a*b > Got: Finitely presented group < a, b  a^7, b^2, (a*b)^2 >
The patch should be rebased to the latest sage beta (5.11.beta3 as of today)
Minor nitpick: Don't end docstrings with a blank line. It just wastes vertical space needlessly. The """
end marker is enough visual cue that the docstring is finished.
comment:6 Changed 8 years ago by
Fixed doctest failures caused by the new printing of free group elements. (And removed unnecessary blank lines)
comment:7 Changed 8 years ago by
I get a big fat warning that libgap is being loaded on Sage startup. You should lazy_import
in sage/groups/groups_catalog.py
.
comment:8 Changed 8 years ago by
The following code placed in sage/groups/groups_catalog.py
:
from sage.misc.lazy_import import lazy_import lazy_import( 'sage.groups', 'finitely_presented_catalog', 'presentation' )
Has the desired effect (libgap is no longer loaded on startup), however groups.lazy_import
is now part of the global namespace. Any suggestions for removing this sideeffect?
comment:9 Changed 8 years ago by
How about this change instead:

sage/groups/all.py
diff git a/sage/groups/all.py b/sage/groups/all.py
a b 21 21 lazy_import('sage.groups.affine_gps.affine_group', 'AffineGroup') 22 22 lazy_import('sage.groups.affine_gps.euclidean_group', 'EuclideanGroup') 23 23 24 import groups_catalog as groups 24 lazy_import('sage.groups', 'groups_catalog', 'groups')
comment:10 Changed 8 years ago by
Yes, the whole groups catalog should be lazy.
comment:11 Changed 8 years ago by
Just made the groups_catalog
import in sage/groups/all.py
lazy. Added some documentation and more tests.
comment:12 Changed 8 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
Looks good to me
comment:13 Changed 8 years ago by
 Status changed from positive_review to needs_work
The patch needs a proper commit message.
comment:14 Changed 8 years ago by
There is a conflict with #14910: both patches add the same reference [TWGT]
.
comment:15 Changed 8 years ago by
 Status changed from needs_work to needs_review
Added commit message, fixed reference conflict.
Changed 8 years ago by
comment:16 Changed 8 years ago by
 Reviewers changed from Volker Braun to Volker Braun, Travis Scrimshaw
Hey,
Here's a small review patch which fixes a few typos and formatting in the documentation, and if you're happy with my changes, we can set this (back to) positive review.
Best,
Travis
comment:17 Changed 8 years ago by
Looks good to me, thank you for the catches.
comment:18 Changed 8 years ago by
 Status changed from needs_review to positive_review
I'm going to interpret that as a positive review.
comment:19 Changed 8 years ago by
 Description modified (diff)
comment:20 Changed 8 years ago by
 Description modified (diff)
comment:21 Changed 8 years ago by
For patchbot:
Apply: trac_14791_named_fpgroups.2.patch, trac_14791reviewts.patch
comment:22 Changed 8 years ago by
 Milestone changed from sage5.12 to sage5.13
comment:23 Changed 8 years ago by
 Merged in set to sage5.13.beta0
 Resolution set to fixed
 Status changed from positive_review to closed
Patch