There should be no implications to IsEnumerableSemigroupRep

[fingolfin]

As discussed in gap-system/gap#2818, the following code in Semigroups is problematic:

InstallTrueMethod(IsEnumerableSemigroupRep, IsGroup and IsFinite);

It is problematic because it means that now GAP consider every finite group to be in the filter IsEnumerableSemigroupRep, and hence in the filter IsComponentObjectRep -- even objects which are in positional rep! Clearly, that's wrong.

IMHO, this extends to all implications to IsEnumerableSemigroupRep. A possible fix would be to add a new filter/category/property IsEnumerableSemigroup; then change IsEnumerableSemigroupRep to imply that, and change the above implications to only imply IsEnumerableSemigroup. You might also want or need to change some more places to use IsEnumerableSemigroup instead of IsEnumerableSemigroupRep.

[james-d-mitchell]

Thanks for the report @fingolfin, this is obviously a bad idea. I intend to get rid of IsEnumerableSemigroupRep in the near future anyway, and will certainly fix it at that point. Can the fix wait, or should I do it more quickly?

[fingolfin]

Thank you! Resolving this can wait, the immediate issue it caused also revealed bad code on the GAP side, which is now fixed, so the immediate problem there is resolved now.

[fingolfin]

Just for the record, it would be nice if there was some progress on this in time for GAP 4.11, which is hope we can get out in April/May or so. But if that's not feasible, well, so be it, we'll manage.

[james-d-mitchell]

I'll try my best to meet the May deadline, and will update whatever progress I make.

[fingolfin]

My apologies for nagging, but: I just see this was added to the 4.0.0 milestone (though it also has the 3.* label). This sounds as if the bigger change of getting rid of IsEnumerableSemigroupRep might be farther away. Perhaps in the meantime, a smaller fix could be implemented, which just gets rid of the bad implication, as outlined by me above?

I'd be happy to discuss this tomorrow, if needed :-)

[james-d-mitchell]

No need to apologise, my (perhaps baseless) thoughts were that this is not so easy to fix and could (probably) be most easily resolved when I reorganise things for 4.0.0. I'm happy to try to find a resolution sooner than that, I'll try to figure out what to do today.

[james-d-mitchell]

@fingolfin I finally got around to partially resolving this, hopefully this is still in time for GAP 4.11.

[wilfwilson]

@james-d-mitchell I disagree that this is resolved, it's only partially resolved. The issue as a whole is that "There should be no implications to IsEnumerableSemigroupRep", and although you've removed the worst one, there are still others.

[james-d-mitchell]

I agree @wilfwilson, I'll remove the label. I do still intend to fix this properly as soon as I can, but it's dependent on finishing some other things first.

[wilfwilson]

That would be good. I think the intention on the GAP side is to disallow installing a true method for a representation (gap-system/gap#3006). There'd be an error or something if you tried to do it. Such a change won't happen until Semigroups and some other packages are updated, so you don't need to worry that Semigroups is going to suddenly stop working with GAP.

[james-d-mitchell]

Ok, good to know.

[fingolfin]

It still would be nice to get this resolved :-)

[james-d-mitchell]

I'm still planning to resolve this! Just need to find the time!

[james-d-mitchell]

I am close to having this resolved now, sorry for the extended delay, but it was rather more complicated than expected.

[fingolfin]

Just came across this again, and now I see that InstallTrueMethod(IsEnumerableSemigroupRep, IsGroup and IsFinite); is disabled in stable-3.4 at least, which is great!

Alas, there are still some related calls around -- as you are certainly aware; I merely list this here for the convenience of future me, so I don't have to look again :-)

InstallTrueMethod(IsEnumerableSemigroupRep, IsFpSemigroup and IsFinite);
InstallTrueMethod(IsEnumerableSemigroupRep, IsFpMonoid and IsFinite);

InstallTrueMethod(IsEnumerableSemigroupRep,
IsReesMatrixSubsemigroup and IsGeneratorsOfEnumerableSemigroup);

InstallTrueMethod(IsEnumerableSemigroupRep,
IsReesZeroMatrixSubsemigroup and IsGeneratorsOfEnumerableSemigroup);

InstallTrueMethod(IsEnumerableSemigroupRep,
                  IsQuotientSemigroup and IsGeneratorsOfEnumerableSemigroup);

[james-d-mitchell]

Thanks @fingolfin, this will be removed altogether in v4.0.0 which I'm hoping to release this month!

[james-d-mitchell]

I'm happy to report that after only 3.5 years, this is finally resolved in v4.0.0. Thanks for your patience @fingolfin