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

Fix isomorphisms from fp monoid to fp semigroup #1039

Merged
merged 3 commits into from
Jan 3, 2017

Conversation

james-d-mitchell
Copy link
Contributor

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

  • Adds new features
  • Improves and extends functionality
  • Fixes bugs that could lead to crashes
  • Fixes bugs that could lead to incorrect results
  • Fixes bugs that could lead to break loops

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

There are two main changes in this PR:

  1. Fix a bug in the method for IsomorphismFpSemigroup for a finitely presented monoid. Previously the inverse of the returned isomorphism was not properly defined (and not the inverse of the isomorphism)

  2. Change the declaration of IsomorphismFpMonoid so that it can be applied to objects in IsSemigroup rather than only IsMonoid. A semigroup can be isomorphic to a monoid, while not being a monoid in the technical GAP sense. For example, the semigroup generated by
    Transformation([1, 2, 3, 3, 3]) is not a monoid in the GAP sense but is mathematically. With this change we could now install methods for IsomorphismFpMonoid for such a semigroup.

Previously the inverse of the returned isomorphism was not properly
defined (and not the inverse of the isomorphism), I rewrote the function
while fixing it. Also added a test which highlighted the previous
problem.
A semigroup can be isomorphic to a monoid, while not being a monoid in
the technical GAP sense. For example, the semigroup generated by
Transformation([1, 2, 3, 3, 3]) is not a monoid in the GAP sense but is
mathematically.  With this change we could now install methods for
IsomorphismFpMonoid for such a semigroup.
@james-d-mitchell
Copy link
Contributor Author

The first commit in this PR is whitespace changes only. I should not have done this since the functional changes are hidden in all of the rewriting, but I did not find time to redo this since I first found this bug and suggested this fix in June.

@codecov-io
Copy link

Current coverage is 49.60% (diff: 97.50%)

Merging #1039 into master will increase coverage by 0.04%

@@             master      #1039   diff @@
==========================================
  Files           424        424          
  Lines        223277     223278     +1   
  Methods        3430       3430          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits         110656     110765   +109   
+ Misses       112621     112513   -108   
  Partials          0          0          

Powered by Codecov. Last update 4ff9b38...316ee56

@james-d-mitchell james-d-mitchell changed the title Fix iso fpsemi Fix isomorphisms from fp monoid to fp semigroup Dec 23, 2016
@fingolfin fingolfin merged commit 7bdb207 into gap-system:master Jan 3, 2017
@fingolfin
Copy link
Member

Thanks!

And you kept the whitespace changes in a separate commit, so it was possible to review the other two commits without being distracted by them. I think that's fine.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Jan 3, 2017
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

Successfully merging this pull request may close these issues.

4 participants