FreeMonoid(0) is a monoid, not a group

[markuspf]

Please make sure that this pull request:

• is submitted to the correct branch (the stable branch is only for bugfixes)
• contains an accurate description of changes for the release notes below
• provides new tests or relies on existing ones
• correctly refers to other issues and related pull requests

Tick all what applies to this pull request

• Improves and extends functionality

Write below the description of changes (for the release notes)

FreeMonoid(0) was previously inferred to be a group which meant that code for monoids needed to be special cased. See also #673.

[markuspf]

Of course this breaks all semigroup tests.

[markuspf]

@james-d-mitchell any input on how your preferred fix looks like?

[james-d-mitchell]

Will have a look later today. I recall trying to fix this previously and
not finding a good solution.
On Tue, 15 Nov 2016 at 13:42, Markus Pfeiffer notifications@github.com
wrote:

@james-d-mitchell https://github.com/james-d-mitchell any input on how
your preferred fix looks like?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#950 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGYFd3W39BP7Cobfo3e2fwGxrMZvhZnuks5q-ba4gaJpZM4Kyfx3
.[image: Web Bug from
https://github.com/notifications/beacon/AGYFd06DNP-ADxC6bchLtF_zwlwQh0GHks5q-ba4gaJpZM4Kyfx3.gif]
{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/gap-system/gap","title":"gap-system/gap","subtitle":"GitHub
repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png
","avatar_image_url":"
https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open
in GitHub","url":"https://github.com/gap-system/gap"}},"updates":{"snippets":[{"icon":"PERSON","message":"@markuspf
in #950: @james-d-mitchell any input on how your preferred fix looks
like?"}],"action":{"name":"View Pull Request","url":"
https://github.com/gap-system/gap/pull/950#issuecomment-260643938"}}}

[james-d-mitchell]

I think the changes you've made are correct, and are the ones I would have made had I had more courage.

The diffs in the tests look pretty harmless to me, I'm going to try running the Semigroups package tests with your changes to see if anything more serious comes up. I'll report when I've done that.

[james-d-mitchell]

@markuspf I ran the Semigroups tests and this does not break anything seriously there (somethings are broken but they are all like the failures in the tests here). I think you should rewrite the test output so that the test don't fail, and press ahead with this.

[markuspf]

I will fix this soon.

[markuspf]

Now this of course begs the question what to do about things like

gap> S := Semigroup(PartialPerm([1]));
<trivial partial perm monoid of rank 1 with 1 generator>

Why does this produce a monoid, not a semigroup?

[james-d-mitchell]

Because it is a monoid :) Is this related to the current issue? The Semigroup function makes a monoid out of any generating set which contains the One of its generators.

[codecov-io]

Current coverage is 49.54% (diff: 100%)

Merging #950 into master will increase coverage by 0.06%

@@             master       #950   diff @@
==========================================
  Files           424        424          
  Lines        223152     223158     +6   
  Methods        3430       3430          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits         110415     110561   +146   
+ Misses       112737     112597   -140   
  Partials          0          0          

Powered by Codecov. Last update f88399d...70ea675

[markuspf]

But is that not the same kind of cleverness that we are just trying to fix?

[james-d-mitchell]

Yes, this was probably a mistake, which I'd like to rectify at a later point.

[olexandr-konovalov]

@markuspf as you likely know - with all packages loaded we now have:

########> Diff in /home/gap-jenkins/workspace/GAP-master-test/GAPCOPTS/32build\
/GAPTARGET/install/label/graupius/GAP-master-snapshot/tst/testinstall/semigrp.\
tst:290
# Input is:
S := Semigroup(PartialPerm([1]));
# Expected output:
<trivial partial perm monoid of rank 1 with 1 generator>
# But found:
<trivial partial perm group of rank 1 with 1 generator>
########
########> Diff in /home/gap-jenkins/workspace/GAP-master-test/GAPCOPTS/32build\
/GAPTARGET/install/label/graupius/GAP-master-snapshot/tst/testinstall/semigrp.\
tst:300
# Input is:
S := Semigroup(IdentityTransformation);
# Expected output:
<trivial transformation monoid of degree 0 with 1 generator>
# But found:
<trivial transformation group of degree 0 with 1 generator>
########
########> Diff in /home/gap-jenkins/workspace/GAP-master-test/GAPCOPTS/32build\
/GAPTARGET/install/label/graupius/GAP-master-snapshot/tst/testinstall/semigrp.\
tst:309
# Input is:
S := Monoid(PartialPerm([1]));
# Expected output:
<trivial partial perm monoid of rank 1 with 1 generator>
# But found:
<trivial partial perm group of rank 1 with 1 generator>
########
########> Diff in /home/gap-jenkins/workspace/GAP-master-test/GAPCOPTS/32build\
/GAPTARGET/install/label/graupius/GAP-master-snapshot/tst/testinstall/semigrp.\
tst:311
# Input is:
S := Monoid(PartialPerm([1]));
# Expected output:
<trivial partial perm monoid of rank 1 with 1 generator>
# But found:
<trivial partial perm group of rank 1 with 1 generator>
########
########> Diff in /home/gap-jenkins/workspace/GAP-master-test/GAPCOPTS/32build\
/GAPTARGET/install/label/graupius/GAP-master-snapshot/tst/testinstall/semigrp.\
tst:317
# Input is:
S := Monoid(IdentityTransformation);
# Expected output:
<trivial transformation monoid of degree 0 with 1 generator>
# But found:
<trivial transformation group of degree 0 with 1 generator>
########

and

########> Diff in [ "./trans.xml", 968, 980 ]
# Input is:
S := Semigroup( IdentityTransformation );
# Expected output:
<trivial transformation group of degree 0 with 1 generator>
# But found:
<trivial transformation monoid of degree 0 with 1 generator>
########
########> Diff in [ "./trans.xml", 1004, 1016 ]
# Input is:
S := Semigroup( IdentityTransformation );
# Expected output:
<trivial transformation group of degree 0 with 1 generator>
# But found:
<trivial transformation monoid of degree 0 with 1 generator>
########
########> Diff in [ "./trans.xml", 1040, 1052 ]
# Input is:
S := Semigroup( IdentityTransformation );
# Expected output:
<trivial transformation group of degree 0 with 1 generator>
# But found:
<trivial transformation monoid of degree 0 with 1 generator>
########

[markuspf]

Most of this is of course @james-d-mitchell's fault and a bug in the semigroups package.

(The other is the documentation not being updated)

[james-d-mitchell]

I don't know if it's my fault, but certainly I am preparing a new version of Semigroups to deal with these changes.