From a8eba2c2a6f9709a1f24daef7f2a5e7369125c92 Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 21 Mar 2018 14:11:11 +0100
Subject: [PATCH] Not all finitely generated magmas are groups

... hence we should not set IsFinitelyGeneratedGroup for them.
Instead, only set IsFinitelyGeneratedGroup if the family indicates
that its elements are associative.

Note that this all is only done to avoid a call to the default
immediate method for IsFinitelyGeneratedGroup.
---
 lib/magma.gi                                       | 14 +++++++++-----
 .../2018-03-21-MagmaWithInversesByGenerators.tst   | 12 ++++++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)
 create mode 100644 tst/testbugfix/2018-03-21-MagmaWithInversesByGenerators.tst

diff --git a/lib/magma.gi b/lib/magma.gi
index 8deedc4ca1..2493edf48a 100644
--- a/lib/magma.gi
+++ b/lib/magma.gi
@@ -652,13 +652,17 @@ InstallOtherMethod( MagmaWithOneByGenerators,
 #M  MagmaWithInversesByGenerators( <gens> ) . . . . . . . .  for a collection
 ##
 MakeMagmaWithInversesByFiniteGenerators:=function(family,gens)
-local M;
+local M, filter;
   if not IsBound(family!.defaultMagmaWithInversesByGeneratorsType) then
+    filter := IsMagmaWithInverses and IsAttributeStoringRep
+                and HasGeneratorsOfMagmaWithInverses;
+    # check if the family implies associativity, and hence that the
+    # final magma is a group
+    if IS_SUBSET_FLAGS(family!.IMP_FLAGS, FLAGS_FILTER(IsAssociativeElementCollection)) then
+       filter := filter and IsFinitelyGeneratedGroup;
+    fi;
     family!.defaultMagmaWithInversesByGeneratorsType :=
-      NewType( FamilyObj( gens ),
-                IsMagmaWithInverses and IsAttributeStoringRep 
-                and HasGeneratorsOfMagmaWithInverses 
-                and IsFinitelyGeneratedGroup);
+      NewType( FamilyObj( gens ), filter );
   fi;
 
   M:=rec();
diff --git a/tst/testbugfix/2018-03-21-MagmaWithInversesByGenerators.tst b/tst/testbugfix/2018-03-21-MagmaWithInversesByGenerators.tst
new file mode 100644
index 0000000000..dc68c6b5b7
--- /dev/null
+++ b/tst/testbugfix/2018-03-21-MagmaWithInversesByGenerators.tst
@@ -0,0 +1,12 @@
+# GAP used to set the IsFinitelyGeneratedGroup property for every finitely
+# generated magma-with-inverses, even those which are not groups.
+gap> A:=[[1,2,3,4],[2,1,4,2],[3,4,1,3],[4,2,3,1]];
+[ [ 1, 2, 3, 4 ], [ 2, 1, 4, 2 ], [ 3, 4, 1, 3 ], [ 4, 2, 3, 1 ] ]
+gap> M:=MagmaWithInversesByMultiplicationTable(A);
+<magma-with-inverses with 4 generators>
+gap> IsFinitelyGeneratedGroup(M);
+Error, no method found! For debugging hints type ?Recovery from NoMethodFound
+Error, no 2nd choice method found for `IsFinitelyGeneratedGroup' on 1 argument\
+s
+gap> IsGroup(M);
+false