deal with FpGroups without IsFinite flag
#5946
Conversation
- add a remark about the design decision that calling functions such as `ConjugacyClasses` should not automatically trigger a `IsFinite` call, - add a `ConjugacyClasses` method for an `FpGroup` without the `IsFinite` flag, such that a better error message is printed
ThomasBreuer
added
kind: enhancement
|
|
||
| In fact, | ||
| calling a function such as <Ref Attr="ConjugacyClasses" Label="attribute"/> | ||
| with an FpGroup that does not store that it is finite |
There was a problem hiding this comment.
Maybe “does not already know” would be slightly better than “does not store” since the later might indicate that some fp groups might compute that they are finite but just not store that info.
There was a problem hiding this comment.
Also shouldn’t FpGroup be “finitely presented group” everywhere here?
There was a problem hiding this comment.
I think FpGroup is better here as it stands for a specific kind of group implementation. PcGroups are also finitely presented but only allow very specific presentations and use special algorithms..
There was a problem hiding this comment.
Concerning "FpGroup":
The manual chapter starts with the sentence: "A finitely presented group (in short: FpGroup) is [...]".
And then FpGroup is used in many places. Following Max's argument, one could even replace more occurrences of "finitely presented groups" by "FpGroup".
Concerning "know" vs. "store":
My interpretation is that "store" expresses the technical fact that the flag is set, whereas "know" is more vague in the sense that inside an algorithm, at some point the value of the flag can be deduced from the given information, but it is not set in the object.
There was a problem hiding this comment.
Good point about "FpGroup", the intention I suppose is that FpGroup is written whenever you mean "group belonging to the category IsFpGroup". Which sounds fine to me.
Concerning "know" vs. "store": I agree that "know" is vague, but I think "store" is worse, because it implies that there are finitely presented groups G where IsFinite(G) can been computed but not stored. If the intention is to say "FpGroups where HasIsFinite returns false" then why not say this explicitly?
|
|
||
| In fact, | ||
| calling a function such as <Ref Attr="ConjugacyClasses" Label="attribute"/> | ||
| with an FpGroup that does not store that it is finite |
There was a problem hiding this comment.
I think FpGroup is better here as it stands for a specific kind of group implementation. PcGroups are also finitely presented but only allow very specific presentations and use special algorithms..
| In fact, | ||
| calling a function such as <Ref Attr="ConjugacyClasses" Label="attribute"/> | ||
| with an FpGroup that does not store that it is finite | ||
| may result in an error message. |
There was a problem hiding this comment.
I think this should explain why this is done. Otherwise a user will rightfully wonder "why do they tell me to enter IsFinite(G) instead of just doing that for me?"
| "in the Reference Manual for the background.\n", | ||
| "Perhaps you want to replace <G> by a group of another type.\n", | ||
| "If you want to continue with the given <G> then\n", | ||
| "you can call 'IsFinite( G );' and then enter 'return;'" ); |
There was a problem hiding this comment.
Should this perhaps include a warning that IsFinite(G) may not terminate?
Yes.
|
| if HasIsFinite( G ) then | ||
| if IsFinite( G ) then | ||
| # Something went wrong, we should not get here. | ||
| ErrorNoReturn( "there should be a higher ranked method" ); |
There was a problem hiding this comment.
Isn't this a check that you as developers of GAP have not made a mistake? If so, shouldn't this be an assertion, rather than an error? If I was confronted with the error message, I wouldn't know what I was supposed to do to resolve it.
There was a problem hiding this comment.
O.k., I will turn this into an assertion.
|
One final comment: shouldn't there be some tests for this? |
The discussion of #5944 came to the conclusion that there is a design decision that operations for f.p. groups should not automatically call
IsFinite--the user should explicitly call (or set)IsFinite.I did not find a statement of such a design decision in the documentation, therefore this pull request proposes adding a paragraph about it to the manual chapter on f.p. groups.
Further, the "no method found error" which one gets is not appropriate; note that the error disappears if the group knows that it is finite, which is confusing for users.
Therefore this pull request proposes a method that gives a better error message in case of the operation
ConjugacyClasses.There are other operations for which the same treatment would be appropriate, otherwise the term "design decision" would not make sense. (
IsDihedralGroupis one example.)Similar methods should get installed for them, ideally in a generic way.
Unfortunately, it is not that easy to determine the operations for which the design decision really holds, and for which methods showing a nicer error message should get installed:
In fact, many operations have methods that just take a group without
IsFiniteflag, and callIsFiniteat some point.And if the operation is declared for finite groups then there are already fallback methods that check
IsFinite.It would be good if the operatons for which the design decision holds could be determined automatically, for example in order to give the Oscar system enough information to catch the errors in advance.