-
Notifications
You must be signed in to change notification settings - Fork 36
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
Better inverses #1021
Better inverses #1021
Conversation
…or the argument of RightGreensMultiplierNC.
…e is no one but the method should work
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
gap/greens/froidure-pin.gi
Outdated
if path = fail then | ||
# This can occur when, for example, a = b and S is not a monoid. | ||
return One(gens); | ||
if IsMultiplicativeElement(a) and IsMultiplicativeElement(b) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this does exactly what you want it to, maybe you meant not IsMultiplicativeElementWithOne(a)
and similar for b
?
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still missing, and the documentation for Left/RightGreensMultiplier should be updated to include:
If S is a semigroup but not a monoid, then the return value is either of the same type as the elements of S or an adjoined identity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing "WithOne" fixed. The documentation is possibly more delicate than that, as an fp semigroup can be a monoid but GAP cannot recognise it as one. One example is < s1, s2 | s1^2=s1, s2^2=s2, s1s2=s2, s2s1=s2>. s1 is a "one" but GAP does not know that.
gap/greens/froidure-pin.gi
Outdated
if IsMultiplicativeElement(a) and IsMultiplicativeElement(b) then | ||
return One(gens); | ||
else | ||
return SEMIGROUPS.UniversalFakeOne; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should add some tests for this too?
…to better-inverses
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, a few specific comments, you'll need to:
- add more test for everything
- a non-no-check version of
OneInverseOfSemigroupElementNC
- doc for
OneInverseOfSemigroupElement
- fix
IsMultiplicativeElementWithOne
in the other place indicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And tests for code coverage as indicated during our meeting just now using:
etc/code-coverage-test-gap.py --gap-root /Users/jdm/gap tst/standard/main/froidure-pin.tst
from inside the semigroups package directory where you are doing your development.
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know which chapter of the documentation this should go to.
Also, might expand the description if needed. example?
gap/greens/froidure-pin.gi
Outdated
if path = fail then | ||
# This can occur when, for example, a = b and S is not a monoid. | ||
return One(gens); | ||
if IsMultiplicativeElement(a) and IsMultiplicativeElement(b) then |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
currently no test for finding inverse of a semigroup that cannot use Froidure-Pin. |
<Description> | ||
<C>OneInverseOfSemigroupElement</C> returns one inverse of the element | ||
<C>x</C> in the semigroup <A>S</A>. | ||
</Description> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example please!
found one using Semigroup(SEMIGROUPS.UniversalFakeOne). Resolved |
doc/attr.xml
Outdated
<Returns> One inverse of an element of a semigroup.</Returns> | ||
<Description> | ||
<C>OneInverseOfSemigroupElement</C> returns one inverse of the element | ||
<C>x</C> in the semigroup <A>S</A>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if there is no inverse?
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, sorry for the long delay!
Add a better method for finding inverses of a multiplicative element of a semigroup that can use Froidure-Pin.
Also add a new method called OneInverseOfSemigroupElement.