Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with ClosureInverseMonoid #435

Closed
ChristopherRussell opened this issue Jan 10, 2018 · 5 comments
Closed

Issue with ClosureInverseMonoid #435

ChristopherRussell opened this issue Jan 10, 2018 · 5 comments
Labels
bug Label for issues or PR which report or fix bugs critical Label for issues or PR that are critical

Comments

@ChristopherRussell
Copy link
Collaborator

ChristopherRussell commented Jan 10, 2018

4cef57f seems to have introduced a bug. I noticed an example where the IdempotentGeneratedSubsemigroup of a semigroup had an element that was not in the semigroup. @mtorpey helped me bisect the history to this commit but we were unsure how the changes were affecting this example.

gap> S := 
> InverseMonoid( [ PartialPerm( [ 1, 4 ], [ 1, 4 ] ), PartialPerm( [ 1, 3, 4, 5 ], [ 1, 6, 4, 2 ] ),
> PartialPerm( [ 1, 2, 4, 6 ], [ 1, 5, 4, 3 ] ), PartialPerm( [ 1, 4 ], [ 4, 1 ] ),
> PartialPerm( [ 1, 3, 4, 5 ], [ 1, 3, 4, 5 ] ), PartialPerm( [ 1, 2, 4, 6 ], [ 1, 2, 4, 6 ] ),
> PartialPerm( [ 1, 2, 3, 4, 5, 6, 7 ], [ 1, 2, 3, 4, 5, 6, 7 ] ) ] );
<inverse partial perm monoid of rank 7 with 7 generators>
gap> I := IdempotentGeneratedSubsemigroup(S);
<inverse partial perm monoid of rank 7 with 4 generators>
gap> I = Semigroup(Idempotents(S));
false
gap> Size(I); Size(Idempotents(S));
5
4
gap> Elements(S);
[ <identity partial perm on [ 1, 4 ]>, (1,4), <identity partial perm on [ 1, 3, 4, 5 ]>,
  [3,6][5,2](1)(4), <identity partial perm on [ 1, 2, 4, 6 ]>, [2,5][6,3](1)(4),
  <identity partial perm on [ 1, 2, 3, 4, 5, 6, 7 ]> ]
gap> Elements(I);
[ <identity partial perm on [ 1, 4 ]>, <identity partial perm on [ 1, 3, 4, 5 ]>,
  <identity partial perm on [ 1, 2, 4, 6 ]>, <identity partial perm on [ 1, 2, 3, 4, 5, 6 ]>,
  <identity partial perm on [ 1, 2, 3, 4, 5, 6, 7 ]> ]
@james-d-mitchell
Copy link
Collaborator

Thanks @ChristopherRussell and @mtorpey, I can't really investigate this this week, but will try next week. Unless you want to have a go at fixing it yourself @ChristopherRussell ?

@ChristopherRussell
Copy link
Collaborator Author

ChristopherRussell commented Jan 10, 2018

I have had a look.

IdempotentGeneratedSubsemigroup(S) calls the following:

InverseSemigroup(Idempotents( S ), rec(
    small := true,
    acting := false ));\

which returns the wrong result which I noted above.

However
InverseSemigroup(Idempotents( S ));
returns the right answer

Does this help?

@ChristopherRussell
Copy link
Collaborator Author

ChristopherRussell commented Jan 10, 2018

And the bug seems to arise in ClosureInverseMonoidOrSemigroupNC where S = One(coll) and coll is the subsemigroup which InverseSemigroup has been called on. The line coll := Filtered(coll, x -> not x in S); removes the identity element of coll and a potentially different element is added back in later because there are many possible identities for a collection of partial perms, in this case.

InstallMethod(ClosureInverseSemigroupOrMonoidNC,
"for a function, an inverse semigroup, finite list of mult. element, and rec",
[IsFunction,
 IsInverseSemigroup and IsGeneratorsOfInverseSemigroup,
 IsMultiplicativeElementCollection and IsFinite and IsList,
 IsRecord],
function(Constructor, S, coll, opts)
  local n, x, T, U, i;

  # EN_SEMI_CLOSURE copies the C++ semigroup if any whenever it is called, so
  # it is essential to filter coll to remove any elements that are already in S
  # here.
  coll := Filtered(coll, x -> not x in S);
  if IsEmpty(coll) then
    return S;
  fi;

  # opts must be copied and processed before calling this function
  # coll must be copied before calling this function

  # Make coll closed under inverses
  coll := Set(coll);
  n    := Length(coll);
  for i in [1 .. n] do
    x := coll[i] ^ -1;
    if not x in coll then
      AddSet(coll, x);
    fi;
  od;

  if Constructor = InverseMonoid and not One(coll) in coll then
    Error("ClosureInverseSemigroupOrMonoidNC2");    
    AddSet(coll, One(coll));
  fi;


@james-d-mitchell james-d-mitchell changed the title Issue from 4cef57f3 Issue with ClosureInverseMonoid Jan 15, 2018
@james-d-mitchell james-d-mitchell added bug Label for issues or PR which report or fix bugs critical Label for issues or PR that are critical 3.0 labels Jan 15, 2018
@james-d-mitchell
Copy link
Collaborator

Think I fixed this @ChristopherRussell can you please comment on the PR?

@wilfwilson
Copy link
Collaborator

This was resolved by PR #436.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label for issues or PR which report or fix bugs critical Label for issues or PR that are critical
Projects
None yet
Development

No branches or pull requests

3 participants