Improve BaseDomain for plist-of-list matrices

[fingolfin]

The added tests also reveal various long standing bugs in the underlying code.

Motivated by PR #4773

[ThomasBreuer]

This looks good.
The change in the BaseDomain method is clearly a bugfix.
The added BaseDomain methods define new defaults, which is fine.
The new BaseDomain method for IsMatrix and IsPlistRep and IsCyclotomicCollColl is consistent with what DefaultScalarDomainOfMatrixList does, but also the BaseDomain method for row vectors should be changed, for the sake of consistency:

gap> m:= [ [ 1 ] ];;
gap> BaseDomain( m );
Rationals
gap> BaseDomain( m[1] );
Integers
gap> DefaultScalarDomainOfMatrixList( [ m ] );
Rationals

I have added comments concerning the tests that are commented out because of the bugs that are present. Shall we merge this pull request, and I prepare a follow-up that deals with the tests that are commented out, and the above inconsistency?

[ThomasBreuer]

Currently Matrix(GF(2), m) returns a copy of m.
The NewMatrix method in question calls ConvertToMatrixRep and ignores the fail result.
I think this method should signal an error.

(Well, we should first agree that we want an error message here.
The documentation states that the matrix entries need not lie in the base ring of the matrix but may also "naturally embed in the sense that addition and multiplication automatically work with elements of the base domain".
The condition about arithmetic operations is satisfied for integers and elements of GF(2) but we are not in the situation of an embedding; perhaps the documentation should be made more precise here.)

[fingolfin]

I think that wording was specifically there based on discussion with @frankluebeck, to allow having entries equal to immediate integer objects like 0 and 1, which are much more efficient than general ring elements. I.e. this was strictly intended for optimization purposes.

However, I always felt this is "iffy", and I would reject it for this constructor. Of course a matrix type may use integer objects internally, to fill entries (as, indeed, the new matrix type over Z/nZ defined by @hulpke does); and the implementor may also provide special constructors which accept as input such immediate integers (and anything else that they see fit), but IMHO the default methods should not be required to do so...

Of course checking whether elements are inside a given domain can be expensive. So perhaps we'd want a (set of) MatrixNC methods which don't perform these checks (and/or a "nocheck" option, or whatever). But arguably the default ways to construct MatrixObj instances should IMHO reject "obvious" nonsense.

[hulpke]

What is the allowed relation between BaseDomain and `DefaultFieldOfMatrix'? What should a function use if only a ring of coefficients is needed?

[ThomasBreuer]

@hulpke If the question is about creating matrix objects, one is on the safe side if one sets the BaseDomain explicitly, like in Matrix( Integers, [ [ 1 ] ] ).
If one creates a matrix object without an argument that specifies the BaseDomain, like in Matrix( [ [ 1 ] ] ), one has to live with the default that is chosen; in this case, one gets a matrix over Rationals.
If the question is about what to do with a given matrix object, DefaultFieldOfMatrix gives just some default domain, whereas BaseDomain is stored in the object and should be assumed to be intentional.

[fingolfin]

Also, DefaultFieldOfMatrix must always be a ring by its name and its documentation. Of course one can cheat and let it return anything, but that's off-spec and I am sure it'll cause troubles down the road.

[fingolfin]

The test failure is caused by the semigroups package. Without it:

gap> Matrix(Integers, [[1,2],[3,4]]);
<2x2-matrix over Integers>
gap> BaseDomain(last);
Integers

With semigroups loaded:

gap> Matrix(Integers, [[1,2],[3,4]]);
Matrix(IsIntegerMatrix, [[1, 2], [3, 4]])
gap> BaseDomain(last);
Error, no method found!

See semigroups/Semigroups#807