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

Missing methods for IsCentral #1795

Closed
olexandr-konovalov opened this issue Oct 20, 2017 · 2 comments
Closed

Missing methods for IsCentral #1795

olexandr-konovalov opened this issue Oct 20, 2017 · 2 comments
Labels
good first issue Issues that can be understood and addressed by newcomers to GAP development kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements
Milestone

Comments

@olexandr-konovalov
Copy link
Member

olexandr-konovalov commented Oct 20, 2017

I've got an example from a user:

gap> M:=DihedralGroup(16);
<pc group of size 16 with 4 generators>
gap> e:=Elements(M)[4];
f3
gap> IsCentral(M,e);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsCentral' on 2 arguments called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
called from read-eval loop at line 23 of *stdin*
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk>

Indeed, the manual says that in IsCentral( M, obj ) the 2nd argument must either be an element or a magma, but there seem no methods installed for this case at all:

brk> ShowMethods();
#I  Searching Method for IsCentral with 2 arguments:
#I  Total: 9 entries
#I  Method 1: ``IsCentral: for two associative FLMLORs-with-one'', value: 86
#I  Method 2: ``IsCentral: for two associative FLMLORs'', value: 76
#I  Method 3: ``IsCentral: for two associative rings-with-one'', value: 70
#I  Method 4: ``IsCentral: generic method for two groups'', value: 70
#I  Method 5: ``IsCentral: for two FLMLORs'', value: 68
#I  Method 6: ``IsCentral: for two associative rings'', value: 60
#I  Method 7: ``IsCentral: for two magmas-with-inverses'', value: 34
#I  Method 8: ``IsCentral: for two magmas-with-one'', value: 28
#I  Method 9: ``IsCentral: for two magmas'', value: 22

I've advised to use e in Centre(M) instead, but I think that a missing method should be added as well. Moreover, Centre is defined as an attribute for magmas.

@stevelinton
Copy link
Contributor

Sounds sensible. Do we need to worry about deciding whether we want Centre or LieCentre?

@fingolfin
Copy link
Member

We are not taking about just one missing method here, though; rather, we'd potentially need one for each of the cases listed by ShowMethods above. Note that all but one of the existing IsCentral methods are implemented using IsCentralFromGenerators (the one exceptions is for groups).
So, we could add a companion to IsCentralFromGenerators, say IsCentralElementFromGenerators, and then use that to add the missing methods (plus one extra method for groups.

(Actually, we could probably use IsCentralFromGenerators and IsCentralElementFromGenerators for groups, too. I am not sure why this is not the case already -- I'd expect Comm( u, g ) <> one (which is used in the group implementation) to be slower than g1 * g2 <> g2 * g1 in general (except perhaps for IsPcGroup and IsPcpGroup, but those should then have their own optimized version of these functions anyway).

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements labels Oct 23, 2017
@fingolfin fingolfin added this to the GAP 4.10.0 milestone Nov 8, 2017
@fingolfin fingolfin added the good first issue Issues that can be understood and addressed by newcomers to GAP development label Nov 8, 2017
fingolfin added a commit to fingolfin/gap that referenced this issue Feb 15, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that can be understood and addressed by newcomers to GAP development kind: bug Issues describing general bugs, and PRs fixing them kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements
Projects
None yet
Development

No branches or pull requests

3 participants