Fix PQuotient error for large groups. #5816
Conversation
- Return fail in AbelianPQuotient if not enough generators for current parameters (logord in PQuotient, or #gens in collector of QuotientSystem qs) - Return fail or error in PQuotient for large groups, depending on option noninteractive - Update documentation of PQuotient - Add test for bugfix
|
|
||
| # | ||
| gap> PQuotient( FreeGroup(2), 5, 10, 520 : noninteractive ) <> fail; | ||
| true |
There was a problem hiding this comment.
actually gives false (hence CI fails)
There was a problem hiding this comment.
Yes, this is something I discuss right now with Hulpke in the original issue thread. The function does not behave like I would expect from the documentation. I already added this test case here, because in my opinion the function should behave like this. So in this case, the quotient can be generated by 520 elements, but the algorithm first needs to create a covering group that exceed this limit. Thus it currently throws fail. But actually I would expect it to not throw fail here.
| ## implementation produces an error message if the order of a | ||
| ## <M>p</M>-quotient exceeds <M>p^{256}</M> or <M>p^{<A>logord</A>}</M>, | ||
| ## respectively. | ||
| ## given. If the order of a <M>p</M>-quotient exceeds <M>p^{256}</M> |
There was a problem hiding this comment.
Is 256 an additional limit, or just the default value for logord ?
There was a problem hiding this comment.
it is the default value, not an additional limit (at least what I can tell from the code and how it behaves on input groups whose quotients exceed 256 generators). Probably we should rephrase it. For now, I tried to stay as close to the original documentation as possible
Co-authored-by: Max Horn <max@quendi.de>
Closes #5809
Text for release notes
Fix PQuotient error for large groups: If
logordis too small, we return fail or error depending on optionnoninteractive.