AbelianGroups should be in Groups().Commutative() / Groups().Commutative().Finite()

[nthiery]

Everything is in the title.

This will solve ​http://ask.sagemath.org/question/2954/problem-with-abeliangroupcayley_graph

I made this ticket depend on #10963 (which will be merged a priori soon) to have the nice axioms syntax, but it's not a strong dependency.

[tscrim]

I've added a general semigroup_generators for all monoids, which returns the monoid generators and the unit (and assuming finitely generated). I've also defined a monoid_generators for finite groups in FiniteGroups instead of an if statement as a followup to #16725.

---

New commits:

​0791169	Added monoid generators for general groups.
​e5cd7cc	Added try-except block for group_generators to fallback to monoid_generators.
​c64a5fb	Added message for NotImplementedError for infinitely generated groups.
​796262d	Changed behavior of general group_generators method.
​4606f0e	Fixed category for abelian groups.
​2d58f4f	Implemented a general semigroup_generators() for monoids.
​757c044	Moved monoid_generators to a method for finite_groups.

[tscrim]

Hey Nathann,

Here's somewhat of a followup to #16725. Do you think you'd be willing to do a review? Thanks.

[ncohen]

Yoooooooooo !!

No problem for those tickets, I more or less understand what is going on ! :-)

It looks good but I have two questions:

1) Is there any reason why AbelianGroup? does not appear in groups.<tab> ?

2) When is the code from Groups.ParentMethods.monoid_generators being used ? You removed a couple of lines as this case is handled by the new FiniteGroups.ParentMethods.monoid_generators, but then the code of the first method claims that it raises an exception when self is not a FiniteEnumeratedSet. Couldn't this method be removed too ? I actually do not understand in which situation that fonction worked without using the "if/return" that you removed.

Nathann

[tscrim]

Replying to ncohen:

1) Is there any reason why AbelianGroup? does not appear in groups.<tab> ?

Probably because someone missed it when doing the initial groups catalog. Add it to groups.misc.<tab> here?

2) When is the code from Groups.ParentMethods.monoid_generators being used ? You removed a couple of lines as this case is handled by the new FiniteGroups.ParentMethods.monoid_generators, but then the code of the first method claims that it raises an exception when self is not a FiniteEnumeratedSet. Couldn't this method be removed too ? I actually do not understand in which situation that fonction worked without using the "if/return" that you removed.

Groups.ParentMethods.monoid_generators is used for any parent in the category of Groups that doesn't implement its own monoid_generators. If a group is in FiniteGroups, which is what the if statement was checking, then (with this change) it calls the FiniteGroups.ParentMethods.monoid_generators instead (it's closer to the front of the MRO). It's a cleaner design (and something I should've realized on #16725).

[ncohen]

Yooooooooo !

Probably because someone missed it when doing the initial groups catalog. Add it to groups.misc.<tab> here?

Well, here or in another ticket. It just takes one lie anyway.

Groups.ParentMethods.monoid_generators is used for any parent in the category of Groups that doesn't implement its own monoid_generators. If a group is in FiniteGroups, which is what the if statement was checking, then (with this change) it calls the FiniteGroups.ParentMethods.monoid_generators instead (it's closer to the front of the MRO). It's a cleaner design (and something I should've realized on #16725).

Yes, but when will the code of Groups.ParentMethods.monoid_generators be used, if not for finite group ? With you patch all FiniteGroups have their own implementation now, but which kind of group is not a FiniteGroup but is a FiniteEnumeratedSet ?

Nathann

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​e8f3aaa	Merge branch 'public/groups/abelian_groups_category-15140' of trac.sagemath.org:sage into public/groups/abelian_groups_category-15140
​6fd6eda	Added multiplicative abelian groups to the catalog.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​eb3696e

Merge branch 'develop' into public/groups/abelian_groups_category-15140

[tscrim]

Replying to ncohen:

Well, here or in another ticket. It just takes one lie anyway.

Done here.

Yes, but when will the code of Groups.ParentMethods.monoid_generators be used, if not for finite group ? With you patch all FiniteGroups have their own implementation now, but which kind of group is not a FiniteGroup but is a FiniteEnumeratedSet ?

Ah, I see the confusion. I'm checking the group generators are in FiniteEnumeratedSets, not the group itself. In other words, I'm checking to see if it's finitely generated (since there is no axiom currently for this).

[ncohen]

Yo !

Done here.

Thanks !

Ah, I see the confusion. I'm checking the group generators are in FiniteEnumeratedSets, not the group itself. In other words, I'm checking to see if it's finitely generated (since there is no axiom currently for this).

Sorry T_T

One last check and I will set it to positive_review in 5 minutes.

Nathann

[ncohen]

Hello again !

Sorry for the delay, it took longer than expected: I checked the doctests in groups/ and I saw that one was broken, so I had to start a "ptestlong" to find out if there were others. I added a commit at public/15140 that fixes them.

Positive review to your code, and as I added a commit it is your turn to review it and change the ticket's status.

Thanks !

Nathann

[tscrim]

LGTM. Thanks Nathann.

PS - I made the branch public so you (or any one else) could just have pushed to it.

---

New commits:

​c127173

trac #15140: Broken doctests