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

Fix Earns to always returns a list, as documented (and other improvements) #4759

Merged
merged 6 commits into from
Feb 9, 2022

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Feb 8, 2022

Recent improvements:

  • The Knuth-Bendix uses a DAG for identifying which rule could apply at a given position. This cleans up other ad-hoc improvements, resulting in shorter and cleaner code
  • The requested fix for Earns to agree with its definition.
  • SubgroupConditionAbove works for arbitrary groups, not just permutation groups
  • LowLayerSubgroups uses orbit in favor of pairwise conjugacy, if this is a plausible approach.
  • Info statement in Dixon-Schneider prints more compact information.
  • Clean up code duplication in compuattion of rewriting system for simple groups

Text for release notes

As promised in the manual, Earns now always returns a list.

@hulpke hulpke added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: performance bugs or enhancements related to performance (improvements or regressions) release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Feb 8, 2022
@hulpke hulpke force-pushed the additions branch 2 times, most recently from 83e406a to c444647 Compare February 8, 2022 22:27
Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix for Earns is not complete. An empty list is now given instead of fail (which is good), but the other case still returns an actual group, rather than a list containing one group (which is what the documentation suggests).

gap> Earns(PrimitiveGroup(8, 3));
Group([ (1,7)(2,8)(3,5)(4,6), (1,3)(2,4)(5,7)(6,8), (1,2)(3,4)(5,6)(7,8) ])

@hulpke
Copy link
Contributor Author

hulpke commented Feb 9, 2022

Earns(PrimitiveGroup(8, 3));

Thank you @wilfwilson -- did not realize that was part of the issue. Will fix.

@hulpke hulpke added release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes and removed release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Feb 9, 2022
method for `RepresentativeAction` has been defined (i.e. pc and perm)

and store data/no need for short words in `NormalSubgroups`
and subsequent changes. This fixes gap-system#4744
Special case more applicable.
Remove duplicate rules, improve borel action.
Also clean up broad code duplication.
Utility functions/data structure for finding rewriting rule that would apply
to word at give position. This is faster (and cleaner) than the
caching/sorting kludges used before.

If code manipulates the `tzrules` entry (as the `fr` package does), the
functionality is disabled.
subsequent manual example
@hulpke hulpke merged commit 3b8efcd into gap-system:master Feb 9, 2022
@fingolfin fingolfin changed the title Knuth-Bendix, Earns and some code cleanup Fix Earns to always returns a list, as documented Aug 17, 2022
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes kind: bug Issues describing general bugs, and PRs fixing them release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Aug 17, 2022
@fingolfin fingolfin changed the title Fix Earns to always returns a list, as documented Fix Earns to always returns a list, as documented (and other improvements) Aug 17, 2022
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: performance bugs or enhancements related to performance (improvements or regressions) release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants