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 unexpected error when testing permutation groups for conjugacy #3747

Merged
merged 3 commits into from
Nov 20, 2019

Conversation

hulpke
Copy link
Contributor

@hulpke hulpke commented Nov 19, 2019

This is a work-around for the problem encountered in #3446, that is the code accessing subs.reps[o] is not executed, if it is not in the expected form (true or a list).

In the cases I encountered this resolves the error (and leads to a correct result).

I however do not understand the code (and parameters) well enough to assure that this is simply an omission in the original code, and not a reflection of a deeper problem. Thge code thus issues a warning, if this happens.
I have been running my own calculations with this modification for half a year now and did not encounter any problems, I thus want to offer it to the general system.

Resolves #3446

@hulpke hulpke added the kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them label Nov 19, 2019
@ChrisJefferson
Copy link
Contributor

This is also my best guess as to how to fix this too (and I'm also unsure)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 84.521% when pulling 9755bf3 on hulpke:fixes into 77a7c42 on gap-system:master.

@fingolfin
Copy link
Member

Thank you!
I have absolutely no idea what's going on in that code, but if both of you think it's plausibel, I fully trust that (and should there be any unexpected regression, we'll deal with that as usual shrug).

@fingolfin fingolfin changed the title Workaround for #3446 Fix unexpected error when testing permutation groups for conjugacy Nov 20, 2019
@fingolfin fingolfin added 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 topic: library labels Nov 20, 2019
@fingolfin
Copy link
Member

Should this be backported? Normally I'd say "no, to risky, let's first test it for a while in master", but since @hulpke has been using it for half a year anyway, it seems not so bad... Thoughts?

@fingolfin fingolfin merged commit b71ad24 into gap-system:master Nov 20, 2019
@hulpke
Copy link
Contributor Author

hulpke commented Nov 20, 2019

@fingolfin
As for backporting, it fixes an issue that has never arisen in 20+ years (and I think I am the only one who encountered it so far) and it gives an error, not a wrong result.

I thus think it is safe to not backport it -- we can always issue a fix later.

@PaulaHaehndel PaulaHaehndel self-assigned this Feb 17, 2021
@PaulaHaehndel PaulaHaehndel added release notes: added PRs introducing changes that have since 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 labels Feb 17, 2021
@PaulaHaehndel PaulaHaehndel removed their assignment Feb 17, 2021
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 kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when testing permutation groups for conjugacy
5 participants