Problem with SubdirectProducts of FP Groups (inter alia)

[stevelinton]

Please use the following template to submit an issue
(you may delete lines which are not used). Thank You!

Observed behaviour

f:=FreeGroup("a", "b");
g:=f/[ f.1^6, f.2^4, f.1^3*f.2^(-2), f.2^(-1)*f.1*f.2*f.1];
Size(g);
g0:=Subgroup(g, [g.1^2]);
g1:=g/g0;
ff:=FreeGroup("c");
h:=ff/[ff.1^12];
h0:=Subgroup(h,[h.1^4]);
h1:=h/h0;
ng:=NaturalHomomorphismByNormalSubgroup(g,g0);
nh:=NaturalHomomorphismByNormalSubgroup(h,h0);
phi:=IsomorphismGroups(g1,h1);
ghom:=CompositionMapping(phi, ng);
hhom:=nh;
nn:=SubdirectProduct(g,h,ghom,hhom);

Gives a no method found for SubdirectProductOp from inside the final call.

Expected behaviour

This should work.

Copy and paste GAP banner (to tell us about your setup)

 ┌───────┐   GAP 4.10dev-1982-g203c7cf of today
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin18.5.0-default64-kv6
 Configuration:  gmp 6.1.2, GASMAN
 Loading the library and packages ...
 Packages:   AClib 1.3.1, Alnuth 3.1.0, AtlasRep 1.5.1, AutoDoc 2018.09.20, 
             AutPGrp 1.10, CRISP 1.4.4, Cryst 4.1.18, CrystCat 1.1.8, 
             CTblLib 1.2.2, FactInt 1.6.2, FGA 1.4.0, GAPDoc 1.6.2, 
             IRREDSOL 1.4, LAGUNA 3.9.0, Polenta 1.3.8, Polycyclic 2.14, 
             PrimGrp 3.3.2, RadiRoot 2.8, ResClasses 4.7.1, SmallGrp 1.3, 
             Sophus 1.24, SpinSym 1.5, TomLib 1.2.7, TransGrp 2.0.4, 
             utils 0.59

Diagnosis

The key method is at lib/gprd.gi:525 this converts the two groups into a common representation and constructs new mappings to take the Subdirect product of the converted groups. These new mappings may not know that they are SingleValued and so do not lie in IsGroupHomomorphism. This makes the method that should apply fail to apply.

The function at line 499 addresses a similar issue, but in this case the problem arises with the InverseGeneralMapping call a bit later. The mappings returned by IsomorphismPcGroup do not have HasIsInjective (which is perhaps the real problem) and so their inverses do not get HasIsSingleValued.

Adding SetIsInjective(hom,true) around line seems to fix the problem and doesn't break testinstall.

[stevelinton]

On a little more reflection, I wonder if there should be a method for SubdirectProductOp which only requires IsGeneralMapping for the third and fourth arguments and then tests the necessary properties, since they are not Categories.