Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Trivial SubmagmaWithOneNC has generators set to a list containing the trivial element #4292

Closed

Conversation

FriedrichRober
Copy link
Contributor

@FriedrichRober FriedrichRober commented Feb 26, 2021

Description

The group returned by TrivialSubgroup had its generators set to an empty list. This is a problem for algorithms that want to access the generators of a group. Therefore I suggest to change the generators to be a list containing the trivial element in this case.

Old Behaviour:

gap> grp := TrivialSubgroup(Group((1,2,3,4),(1,2)));
Group(())
gap> GeneratorsOfGroup(grp);
[  ]
gap> GeneratorsOfGroup(Group(()));
[ () ]

Text for release notes

Calling SubmagmaWithOneNC with an empty list returns a group with generators set to a list containing the trivial element.

Further details

This issue was brought up by @DominikBernhardt in one of my repositories. gap-packages/WPE#14

Checklist for pull request reviewers

  • proper formatting

  • usage of relevant labels

    1. either release notes: not needed or release notes: to be added
    2. at least one of the labels bug or enhancement or new feature
    3. for changes meant to be backported to stable-4.X add the backport-to-4.X label
    4. consider adding any of the labels build system, documentation, kernel, library, tests
  • runnable tests

  • lines changed in commits are sufficiently covered by the tests

  • adequate pull request title

  • well formulated text for release notes

  • relevant documentation updates

  • sensible comments in the code

@FriedrichRober FriedrichRober added kind: bug Issues describing general bugs, and PRs fixing them do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) labels Feb 26, 2021
@FriedrichRober FriedrichRober changed the title Trivial SubmagmaWithOneNC has generators set to a list containing the trivial element WIP: Trivial SubmagmaWithOneNC has generators set to a list containing the trivial element Feb 26, 2021
@FriedrichRober FriedrichRober added kind: quirk Issues that are not bugs, but a discrepancy between user expectation and system behavior and removed kind: bug Issues describing general bugs, and PRs fixing them labels Feb 26, 2021
@ChrisJefferson
Copy link
Contributor

This issue has been discussed previously ( #1239 , #4089 ).

It was decided that having an empty list of generators is acceptable. If you need an element of the group, you can explicitly take the identity.

@FriedrichRober
Copy link
Contributor Author

Thanks for clarifying and linking the related issues.

Since it is already decided that an empty list is acceptable, I will close this pull request.

@FriedrichRober FriedrichRober deleted the fr/TrivialSubgroup branch September 21, 2021 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) kind: quirk Issues that are not bugs, but a discrepancy between user expectation and system behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants