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 PreImagesRepresentative for IsToPcGroupHomomorphismByImages #5705

Conversation

ThomasBreuer
Copy link
Contributor

Added a check whether the given element is in the image of the map. Now the method returns fail if not, as is specified in the documentation of PreImagesRepresentative.

Note that InversePcgs is called (only) in the
PreImagesRepresentative method for a group homomorphism in IsToPcGroupHomomorphismByImages.
It sets the component rangePcgs; the value is a pcgs for the image of the homomorphism (in general not for the range).

Resolves #5704, but there are likely other bugs of this kind.

I expect that #5073 or some follow-up of it will discuss the problem in the text for the release notes, thus there is no need to mention the current pull request in the release notes.

Added a check whether the given element is in the image of the map.
Now the method returns `fail` if not, as is specified in the
documentation of `PreImagesRepresentative`.

Note that `InversePcgs` is called (only) in the
`PreImagesRepresentative` method for a group homomorphism in
`IsToPcGroupHomomorphismByImages`.
It sets the component `rangePcgs`; the value is a pcgs for the *image*
of the homomorphism (in general *not* for the range).
@ThomasBreuer ThomasBreuer added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Apr 22, 2024
Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

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

This change will not cause any correctness problems, but the element test will slow down every pre image computation.
Maybe there needs to be an NC version.

@ThomasBreuer
Copy link
Contributor Author

Maybe there needs to be an NC version.

Yes, see #5073.

@ThomasBreuer ThomasBreuer merged commit 59190ce into gap-system:master Apr 24, 2024
25 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_PreImagesRepresentative_pcgroup branch April 24, 2024 14:16
@fingolfin fingolfin changed the title fix PreImagesRepresentative for IsToPcGroupHomomorphismByImages Fix PreImagesRepresentative for IsToPcGroupHomomorphismByImages May 6, 2024
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: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong group order of a preimage of a group homomorphism
2 participants