Adding the zero element to the generating set for graph inverse semigroups with at most one vertex. - #1216
Adding the zero element to the generating set for graph inverse semigroups with at most one vertex.#1216joe-dw wants to merge 6 commits into
Conversation
|
There are some tests that currently fail as they expect the old behavior, where |
|
@james-d-mitchell this now passes the standard tests on my end, AssignGeneratorVariables still works and warns the user when the generating set includes zero |
| SetGeneratorsOfSemigroup(S, | ||
|
|
||
| if DigraphNrVertices(graph) < 2 then | ||
| SetGeneratorsOfSemigroup(S, | ||
| Concatenation(gens, | ||
| List([1 .. DigraphNrEdges(graph)], | ||
| x -> gens[x] ^ -1), | ||
| [MultiplicativeZero(S)])); | ||
| Add(gens, MultiplicativeZero(S)); | ||
| else | ||
| SetGeneratorsOfSemigroup(S, | ||
| Concatenation(gens, | ||
| List([1 .. DigraphNrEdges(graph)], | ||
| x -> gens[x] ^ -1))); | ||
| fi; | ||
| SetGeneratorsOfInverseSemigroup(S, gens); | ||
| SetGraphOfGraphInverseSemigroup(S, graph); |
There was a problem hiding this comment.
This'd be more compact as (a properly formatted, and working version of):
SetGraphOfGraphInverseSemigroup(S, graph);
inv_gens := [];
for i in [1 .. DigraphNrVertices(graph) + DigraphNrEdges(graph)] do
Add(inv_gens, Objectify(fam!.type, [[i], graph]));
od;
gens := Concatenation(inv_gens, List([1 .. DigraphNrEdges(graph)], x -> gens[x] ^ -1));
if DigraphNrVertices(graph) < 2 then
Add(gens, MultiplicativeZero(S));
Add(inv_gens, MultiplicativeZero(S));
fi;
SetGeneratorsOfInverseSemigroup(inv_gens);
SetGeneratorsOfSemigroup(gens);
if the order of the generators is not important, then you could conditionally add the zero to inv_gens before populating gens. I haven't checked that this actually works.
There was a problem hiding this comment.
I've added it at the end as there are test cases that use the fact that, for any i less than the number of edges,
S.i = e_i
And this way it shouldn't break anything
There was a problem hiding this comment.
I think that makes sense
There was a problem hiding this comment.
Will implement your changes and add some comments
james-d-mitchell
left a comment
There was a problem hiding this comment.
Thanks @joe-dw this looks mostly good, a little refactoring required please. Also can you please add a comment to the code indicating why these changes are being made (something like "if the number of vertices is 1 or 0, then the multiplicative zero element of a gis is not a product of any vertices or edges, and so must be added to both generating sets).
Fixing issue #1211