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

Add DirectProductOfPermGroupsWithMovedPoints helper #4326

Merged
merged 2 commits into from
Mar 25, 2021

Conversation

fingolfin
Copy link
Member

This is basically what DirectProduct does for a set of permutation groups,
except that one can override the moved points for each factor. This is useful
if one needs to compute "sums of G-sets": If e.g. we have a trivial group
acting on 3 points, and a Sym(4), then DirectProduct just gives back the
Sym(4). But if we need the result to act on 3+4=7 points, with the Sym(4)
acting on points acting on [4..7], then this new helper allows us to do that.

For now, I left this deliberately undocumented, as it is quite niche.

Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

Perhaps Immutable should be called on the entries of olds, in order to have independent lists inside the (mutable) DirectProductInfo record.

Copy link
Contributor

@fieker fieker left a comment

Choose a reason for hiding this comment

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

As far as I understand this, this indeed will solve my problem (not only mine I hope)
I think the IsFinite(d1) ca IsFinite(g1) is correct (but irrelevant)

This is basically what DirectProduct does for a set of permutation groups,
except that one can override the moved points for each factor. This is useful
if one needs to compute "sums of G-sets": If e.g. we have a trivial group
acting on 3 points, and a Sym(4), then DirectProduct just gives back the
Sym(4). But if we need the result to act on 3+4=7 points, with the Sym(4)
acting on points acting on [4..7], then this new helper allows us to do that.
@fingolfin
Copy link
Member Author

@wilfwilson fixed the tests, thanks
@ThomasBreuer added a commit which makes some stuff immutable
@fieker indeed it should.

@fingolfin fingolfin added the kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements label Mar 23, 2021
@ThomasBreuer
Copy link
Contributor

The test failure in gap/tst/testbugfix/2006-08-28-t00151.tst:14 looks strange.
My first idea was that garbage collections could distort the timings, but they should not need the major part of the time.
(Are such timing tests safe at all?)

@wilfwilson
Copy link
Member

@ThomasBreuer I know that @hulpke encountered the same failure while working on his PR #4320, which is why he made a change in that PR (now in 52fa8c9) to increase the work done by this test, in an attempt to make it more robust.

@wilfwilson
Copy link
Member

The same kind of test failure was also encountered in #4332; see #4332 (comment). Let's keep an eye out for this happening again.

@wilfwilson wilfwilson added release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library labels Mar 25, 2021
@wilfwilson
Copy link
Member

Since this is (as @fingolfin said) a niche feature backend feature, without documentation, I don't think this needs to be mentioned in the release notes.

@wilfwilson wilfwilson merged commit 4a3fe6a into gap-system:master Mar 25, 2021
@fingolfin fingolfin deleted the mh/dirprod branch March 25, 2021 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants