Fix + and * methods for a DirectProductElement and a non-list collection

[fingolfin]

These are problematic as follows: Suppose you want to know what kind
of object A*B is.

• If A and B are "scalars", it is their sum.
• If A is a "scalar" and B=[x,y] is a list or a direct product element (dpe),
  it is the list [Ax,Ay].
• If A=[x,y] is a list and B is a "scalar", it is the list [xB, yB]
• If both are lists, it is their scalar product

But now what if A is a RightCoset in a group G, and B a group element?
Then we might expect this to be the new right coset A*B, at least if B is in G.

But if B is a list or dpe [x,y], it could also mean the dpe [Ax, Ay].

This ambiguity is the source of problems once the method ranks fluctuate (say as result of LoadAlPackages), as that changes which of the two interpretations above takes effect.

For a specific example, consider this:

gap> G:=DirectProduct(Group(()), CyclicGroup(2));;
gap> RightCoset(G, One(G)) * One(G);
RightCoset(<group with 2 generators>,[ (), <identity> of ... ])

If the automgrp package or loaded, or if we run DeclareFilter("IsFoobar", 100); InstallTrueMethod(IsFoobar, IsFinite); before this code, the last command will instead result in a method not found error.

This PR uses a hack to work around this particular issue, by modifying some methods which currently try to disambiguate the above situation by checkin the arguments against the filter IsList, to instead check IsListOrCollection. That solves the method rank issues I am seeing, but still seems rather dubious to me. For example, what now if one forms the direct product of groups, where one groups really has right cosets as its elements? Right now, that doesn't really seem to work, but in theory, it could be made to work...

I'd appreciate any input on this. I note that @ThomasBreuer committed this code 2002-02-21, so perhaps he has some further insights on this.

[ChrisJefferson]

So there are two parts here:

I agree with your fix, and think it improves things.

In the big picture, it feels hard to "fix" entirely, because as you say, there are expressions where * could mean to different, valid things, and we have to pick one.

[codecov]

Codecov Report

Merging #2869 into master will decrease coverage by 7.54%.
The diff coverage is 75%.

@@             Coverage Diff             @@
##           master    #2869       +/-   ##
===========================================
- Coverage   83.72%   76.18%    -7.55%     
===========================================
  Files         681      479      -202     
  Lines      346559   240469   -106090     
===========================================
- Hits       290166   183201   -106965     
- Misses      56393    57268      +875

Impacted Files	Coverage Δ
lib/tuples.gi	92% <75%> (-3.63%)	⬇️
lib/trans.gd	0% <0%> (-100%)	⬇️
lib/pperm.gd	0% <0%> (-97.02%)	⬇️
hpcgap/lib/hpc/thread.g	0% <0%> (-67.86%)	⬇️
lib/rws.gi	14.7% <0%> (-56.59%)	⬇️
lib/set.gi	9.09% <0%> (-56.4%)	⬇️
lib/rvecempt.gi	37.5% <0%> (-52.3%)	⬇️
hpcgap/demo/cancel.g	0% <0%> (-50%)	⬇️
lib/mgmideal.gi	11.39% <0%> (-48.5%)	⬇️
lib/float.gd	45.45% <0%> (-48.17%)	⬇️
... and 599 more

[wilfwilson]

Ah. This problem had occurred to me before. Like Chris, I agree that this improves things.

[ThomasBreuer]

First of all, the proposed changes correct the methods in question, and I agree with the proposal.

With the code in lib/tuples.gi, also the documentation of IsDirectProductElement
in lib/tuples.gd should be corrected, in the same way.
Up to now, the documentation states that the product (resp. the sum) of an object x, say,
in IsDirectProductElement and an object n, say, that is not a list
is defined as the object in IsDirectProductElement
that consists of the of the products (resp. the sums) of the entries of x with n.
We have to restrict this definition to the case that n is neither a list nor a collection.

Concerning the possibility of groups whose elements are right cosets,
I hope that such constructions are not allowed.
For example, it would be tempting to regard those right cosets that contain the identity element as groups;
if such a coset would also be a (factor) group element then it would have both a size (as a domain)
and an order (as a multiplicative element).
Each of the three possible identifications that occur here
--groups with cosets, cosets with group elements, order with size--
may look natural, but one cannot have all of them.

[olexandr-konovalov]

@fingolfin this seems to fix #2818, as far as I understand from #2835, isn't it?

[fingolfin]

@alex-konovalov Yes, at least it did when I last tested. Of course since then many package updates were released.

@ThomasBreuer I agree with your analysis on "groups with cosets as elements" (my personal view is that it is a mistake that Order is allowed as a synonym for Size if the input is a group -- but that ship has sailed long ago). So, if you agree with the general change in this PR, then indeed the documentation must be updated (I did not want to do that as long as it was completely unclear whether the change in here might be deemed an acceptable fix at all).

[fingolfin]

I now updated this PR as discussed.

[ThomasBreuer]

Very good.

[olexandr-konovalov]

@fingolfin I've updated packages-master.tar.gz on the weekend, and I think that you've tested with it already after that. However, with the freshly build Docker container for the master branch which I've triggered this evening to include this PR, there are still failures at https://travis-ci.org/gap-system/gap-docker-master-testsuite/builds/436339704 when all packages are loaded, although some diffs disappeared indeed.

[fingolfin]

These error look like they are caused by an outdated JupyterKernel version with the broken ErrorInner override.

[olexandr-konovalov]

No, that uses the latest JupyterKernel 0.99999999 from 22/09/2018.