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

Incorrect behaviour for a monoid created by MonoidByAdjoiningIdentity #371

Closed
wilfwilson opened this issue Sep 6, 2017 · 3 comments
Closed

Comments

@wilfwilson
Copy link
Collaborator

wilfwilson commented Sep 6, 2017

In the GAP lib/adjoin.gd file, MonoidByAdjoiningIdentity is declared as an attribute that adjoins an identity to a semigroup (even if it already has one). Without the Semigroups package loaded, this is the following correct behaviour:

gap> R := ReesMatrixSemigroup(Group(()), [[()]]);;
gap> S := MonoidByAdjoiningIdentity(R);
gap> Size(S);
2
gap> Elements(S);
[ ONE, (1,(),1) ]

With the Semigroups package loaded, this is what we get:

gap> R := ReesMatrixSemigroup(Group(()), [[()]]);;
gap> S := MonoidByAdjoiningIdentity(R);
<commutative monoid with 1 generator>
gap> Size(S);
1
gap> Elements(S);
[ (1,(),1) ]

I've not dug into what's going on.

@wilfwilson
Copy link
Collaborator Author

Additional information. Without Semigroups package:

gap> R := ReesMatrixSemigroup(Group(()), [[()]]);
gap> S := MonoidByAdjoiningIdentity(R);
gap> GeneratorsOfSemigroup(S);
[ ONE, (1,(),1) ]

With Semigroups:

gap> R := ReesMatrixSemigroup(Group(()), [[()]]);;
gap> S := MonoidByAdjoiningIdentity(R);
gap> GeneratorsOfSemigroup(S);
[ (1,(),1) ]

@wilfwilson
Copy link
Collaborator Author

wilfwilson commented Sep 13, 2017

Line 280-284 in gap/semigroups/semigrp.gi:

  if CanEasilyCompareElements(gens) and not One(gens) in gens then
    SetGeneratorsOfMagma(S, Concatenation([One(gens)], gens));
  else
    SetGeneratorsOfMagma(S, AsList(gens));
  fi;

In this example, gens is basically [ (1,(),1) ], i.e. it doesn't contain ONE, and moreover, the remaining generator does not generate (using *) this adjoined identity. So really we should be adding ONE at this point too.

@wilfwilson
Copy link
Collaborator Author

@james-d-mitchell shall I just change this code block to:

if not One(gens) in gens then
  SetGeneratorsOfMagma(S, Concatenation([One(gens)], gens));
else
  SetGeneratorsOfMagma(S, AsList(gens));
fi;

?

wilfwilson added a commit to wilfwilson/Semigroups that referenced this issue Sep 13, 2017
wilfwilson added a commit to wilfwilson/Semigroups that referenced this issue Sep 13, 2017
wilfwilson added a commit to wilfwilson/Semigroups that referenced this issue Sep 13, 2017
wilfwilson added a commit to wilfwilson/Semigroups that referenced this issue Sep 13, 2017
flsmith pushed a commit to flsmith/Semigroups that referenced this issue Sep 20, 2023
…entations (semigroups#371)

* Update relations

* Add relation that e_12 is idempotent (missing from book)

* Add test using presentation

* Add relations from full transformation monoid

* Perform error checking and formatting

* Increase degree for ToddCoxeter tests

* Fix errors

* Change test tags

* Add extreme test

* Improve lambda functions

* Perform formatting

* Fix compilation warning

* Add extreme test

* Intermediate progress: things work

* Sort author value

* Change Aizenstat to Sutov, for partial transformation monoid

* Move function to correct unnamed namespace

* Add suggestion to self

* Add comment for function

* Formatting

* Improve comment consistency

* Remove FIXME comment

* Fixup per James's review

* Change not for exclamation mark

* Fix bracket

* Perform formatting

* Update Sims1 test

Co-authored-by: Murray Whyte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants