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 IsTransitive to require that the group actually acts on the given domain (strictly speaking, the previous behavior was "as documented", but it made no sense and led to bugs elsewhere) #4907

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

ThomasBreuer
Copy link
Contributor

@ThomasBreuer ThomasBreuer commented Jun 27, 2022

... in the case that the group does not act on the given domain

Text for release notes

Up to now, IsTransitive( G, D ) did not require that G acts on D.
Thus true was returned also when D was a proper subset of a G-orbit.
From now on, IsTransitive returns true only if the given domain is closed under the G-action.
The documentation has been changed accordingly.
Additionally, the documentation of several operations (ActionHomomorphism, Blocks, MaximalBlocks, RepresentativesMinimalBlocks) has been extended by warnings that GAP assumes that the arguments really describe a group action.

Further details

The old behaviour of IsTransitive was consistent with the old documentation,
but other parts of GAP apparently assumed that a group that acts transitively on a domain really acts on this domain.
(For example RankAction shows an error message that the action must be transitive if the group does not act on the given domain.)

There are related situations where GAP does not check whether the group in question really acts on the domain in question, see issue #4904.

... in the case that the group does not act on the given domain
@ThomasBreuer ThomasBreuer added kind: bug Issues describing general bugs, and PRs fixing them topic: documentation Issues and PRs related to documentation kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them topic: library release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jun 27, 2022
... in order to state explicitly that the result of several operations
is undefined if the arguments do not define a (transitive) group action

(I have also added some tests that document certain situations where
one does not get an immediate error message but invalid objects are
returned, which then lead to error messages later on.
The idea is that it might be necessary to adjust the documentation
as soon as some of these tests fail due to code improvements.)
@fingolfin fingolfin merged commit 17c8d3c into gap-system:master Jun 30, 2022
@ThomasBreuer ThomasBreuer deleted the TB_IsTransitive branch July 1, 2022 08:42
@fingolfin fingolfin removed the topic: documentation Issues and PRs related to documentation label Aug 17, 2022
@fingolfin fingolfin changed the title fixed IsTransitive ... Change IsTransitive to require that the group actually acts on the given domain Aug 17, 2022
@fingolfin fingolfin changed the title Change IsTransitive to require that the group actually acts on the given domain Fix IsTransitive to require that the group actually acts on the given domain (strictly speaking, the previous behavior was "as documented", but it made no sense and led to bugs elsewhere) 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 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: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them 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 topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants