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 a rare error message in isomorphism test #5298

Merged
merged 5 commits into from
Jan 10, 2023
Merged

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Dec 24, 2022

This fixes a bug reported by B. Sambale in the forum on 12/24/2022: When adding to the series, the relevant radical automorphisms were only computed in one case, not both. This will lead to an error message if wrong.

It also contains updates to fitting free setup so that it will work for hybrid groups.

@hulpke hulpke added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them kind: new feature release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Dec 24, 2022
@hulpke hulpke force-pushed the fixes branch 3 times, most recently from 198c5cf to d195728 Compare January 8, 2023 15:01
@fingolfin
Copy link
Member

The testexpect error is most likely unrelated, and appears elsewhere. I'll open an issue to deal with it.

Otherwise this PR looks good to me. However, I would prefer to first merge PR #5275; afterwards this and a a few other PRs may need to be adjusted, which I will take care of myself (i.e. no need for @hulpke to update this or the other PR), and then I hope we can merge both of your PRs soon.

Sorry for the delay.

@hulpke
Copy link
Contributor Author

hulpke commented Jan 9, 2023

The testexpect error is most likely unrelated, and appears elsewhere
That is what I reckoned.

I'll open an issue to deal with it.

Thank you.

Otherwise this PR looks good to me. However, I would prefer to first merge PR #5275;

While I consider this PR (and gaplint) as solutions in search of a problem, I can live with this. Thanks for the update.

Double cosets in Dixon-Schneider can use coset perm action, if degree is
small
use presentation for factor and closure, rather than coKernelGenerators.
This requires fewer products in preimage.
when inserting certain subgroups. This fixes a bug reported by B. Sambale in
the forum on 12/24/2022
can lead no natural homomorphism into formal direct product, not efficient.
This is relevant when forming factor groups as subdirect products.
@fingolfin fingolfin enabled auto-merge (rebase) January 10, 2023 12:12
@fingolfin fingolfin merged commit 8cbcc1f into gap-system:master Jan 10, 2023
@fingolfin fingolfin added 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 and removed release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements kind: new feature release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Jan 28, 2024
@fingolfin
Copy link
Member

This PR apparently contains simultaneously a bug fix, a new feature and enhancements, thus would need multiple entries in the release notes and can't just be handled via the title.

But since also the "release notes: use title" was set, I assume only the bugfix is meant to be in the release notes. I've thus adjusted the labels to only indicate the bug fix, so as to not mislead the script generating the release notes.

@hulpke
Copy link
Contributor Author

hulpke commented Jan 28, 2024

@fingolfin
Yes, that is correct.
I'm sorry that the clustering of items causes a problem. If there wasn't the required approval, I would be most happy to avoid putting items together that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, 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.

2 participants