`Matrix(Integer,[[1]])` returns a `IsIntegerMatrix` but does not implement the `MatrixObj` protocol

[fingolfin]

The following happens with both Semigroups 3.4.2 and 4.0.0:

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!

There are two options to resolve this:

1. implement BaseDomain and possibly other methods needed to make IsIntegerMatrix compliant with the MatrixObj interface (to the best of your ability -- we definitely need to improve the docs, and add conformance tests)
2. don't do this, but then also don't change the behaviour of the Matrix(Integers, M) constructor

Observed in gap-system/gap#4805

[wilfwilson]

This is related to #512.

[wilfwilson]

And I did some unfinished work in this direction in #621.

[fingolfin]

In this particular case, though, just adding a BaseDomain method for IsIntegerMatrix might already help, and shouldn't be hard or cause problems for you, should it? (Granted, I haven't looked at the code at all, so maybe I am mistaken here).

[james-d-mitchell]

I’m happy enough to try to resolve this, but would like to point out that I think this is an issue caused by changes in GAP rather than any change in Semigroups. In particular If I recall the code for creating integer matrices using ‘Matrix’ in Semigroups predates the code in GAP. In some sense this is the GAP library changing the behaviour of Semigroups rather than the other way around. This is just for the record, it’s undoubtedly better to have less code for matrices in Semigroups and more in the GAP library, and I’m happy to make whatever changes might be required.

[fingolfin]

I’m happy enough to try to resolve this

Thanks!

[...], but would like to point out that I think this is an issue caused by changes in GAP rather than any change in Semigroups.

You are 100% right, and I misrepresented this above (not intentionally, but still); in fact earlier today I spent digging through the issue & PR @wilfwilson linked to, re-reading things that I'd forgotten about (including my own comments 😅). It's an annoyingly complex and far too long-unresolved story 😞

In particular If I recall the code for creating integer matrices using ‘Matrix’ in Semigroups predates the code in GAP. In some sense this is the GAP library changing the behaviour of Semigroups rather than the other way around.

Indeed! IMHO the real problem here is that GAP doesn't have namespaces or some other real strategy for encapsulation of modules, and hence clashes (esp. for nice & sweet names like Matrix) are difficult to avoid.

This is just for the record, it’s undoubtedly better to have less code for matrices in Semigroups and more in the GAP library, and I’m happy to make whatever changes might be required.

Thank you!

[james-d-mitchell]

This is related to:

gap-system/gap#4814

Basically, the package (against the master branch of GAP) is completely broken by the changes detailed in:

Recent change to Matrix causes Semigroups package CI to break

[james-d-mitchell]

IMHO the real problem here is that GAP doesn't have namespaces or some other real strategy for encapsulation of modules, and hence clashes (esp. for nice & sweet names like Matrix) are difficult to avoid.

I agree, but we are where we are!

Reading over @wilfwilson's comments in PR #621, I think the "best" thing to do would be to somehow convert the integer and finite field matrices in Semigroups to be MatrixObjs while leaving matrices over other semirings as they are.

I think fixing this for integer matrices might be more or less straightforward, we can just start using GAP's integer matrices in Semigroups. Maybe it won't be that easy, but I can't think of any obstacles right away.

I'm not so sure about matrices over finite fields, it seems that GAP does not allow 0-dimensional matrices over finite fields, and 0-dimensional matrices are crucial for computing semigroups of matrices over finite fields (I don't exactly remember the details but this somehow comes up and was a major point of pain when Markus and I implemented the stuff in Semigroups for semigroups of matrices over finite fields). We could replace all calls to Matrix(GF(2), something) by something else. But this would be backwards incompatible, and I'm not sure I want to do this. I don't really like the idea that our (5 year old) working code can be broken by an upstream change in GAP, and that the onus is on us to make changes to resolve the issue.

I'm game to fix this, but I don't really know how to get started, don't want to break backwards compatibility unless it's absolutely necessary. If anyone has any ideas, I'll be happy to hear them.

[wilfwilson]

Reading over @wilfwilson's comments in PR #621, I think the "best" thing to do would be to somehow convert the integer and finite field matrices in Semigroups to be MatrixObjs while leaving matrices over other semirings as they are.

@james-d-mitchell You might want to take a look at the diff on PR #621 (especially the main EXPERIMENT: commit, which does all the work). The PR goes some way to achieving exactly this. I'll post a comment there with more details.

[james-d-mitchell]

Thanks @wilfwilson I'll take a look as soon as I have time.

[james-d-mitchell]

I believe that this is resolved in v4.0.1, if so I'll close the issue, thoughts @fingolfin and @wilfwilson ?

[fingolfin]

No, this still happens exactly as described, as BaseDomain is still not implemented.

[fingolfin]

I think PR #621 is needed to resolve this