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

Improved direct products, with new supporting methods #347

Merged
merged 6 commits into from Oct 2, 2017

Conversation

wilfwilson
Copy link
Collaborator

@wilfwilson wilfwilson commented Jul 14, 2017

This PR includes several related changes. Most importantly:

  • this PR introduces a new method for DirectProductOp, for a list of either transformation semigroups, or bipartition semigroups, or partial perm semigroups.

Previously, DirectProductOp methods existed for:

  • a list of transformation monoids
  • a list of transformation monoids-as-semigroups
  • an arbitrary list of transformation semigroups
  • a list of partial perm inverse monoids

This PR extends this functionality to support all partial perm semigroups, and all bipartition semigroups. It does this with a unified function. Notably, this function replaces the previous method for arbitrary finite transformation semigroups, which used a brute force search to find a generating set for the direct product.

My new method sometimes returns a semigroup with a larger generating set than the previous method, but it finds the answer far faster. To see an example of improved performance, try calculating the direct product of the singular transformation semigroup of degree 4 with itself. This takes 40ms with the new code, and gives a semigroup with 96 generators. The old code takes 4800ms, although it does only use 53 generators. Try a higher degree and the time difference is far starker. The generators returned by the new method can be reduced by SmallGeneratingSet, or similar, if desired.

I have ideas for reducing the number of generators returned my method, but they're still only ideas, and in any case, they would require my (future) pull request concerning the partial order of L- and R-classes of a semigroup, and in particular, maximal L- and R-classes.

The new direct product code is reliant on the new methods called:

  • NonTrivialFactorization.

Also introduced are the related new functions:

  • IsDecomposableSemigroup (i.e. is S^2 = S?)
  • IndecomposableElements (i.e. those elements in S \ S^2)

The indecomposable elements of S are precisely those elements in GeneratorsOfSemigroup(S) without a non-trivial factorization in the semigroup.

@wilfwilson wilfwilson added the do not merge Label for PR that should not be merged label Jul 14, 2017
@wilfwilson wilfwilson force-pushed the decomposable branch 2 times, most recently from 44119af to f066b03 Compare August 1, 2017 11:24
@wilfwilson wilfwilson force-pushed the decomposable branch 4 times, most recently from 82bd029 to fc752b0 Compare August 30, 2017 21:22
@wilfwilson
Copy link
Collaborator Author

This is almost ready for merging, I just need to complete my proof that the algorithm is correct.

@wilfwilson wilfwilson force-pushed the decomposable branch 3 times, most recently from 844e300 to 7743fe4 Compare September 6, 2017 13:47
@wilfwilson wilfwilson force-pushed the decomposable branch 2 times, most recently from dac7e51 to 10cd103 Compare September 14, 2017 14:04
@wilfwilson wilfwilson removed the do not merge Label for PR that should not be merged label Sep 17, 2017
@wilfwilson
Copy link
Collaborator Author

It took far longer to prove than I thought it would, but I'm now confident that this PR is mathematically sound, and therefore ready to merge. I'd be grateful for reviews!

@wilfwilson
Copy link
Collaborator Author

@mtorpey and I are both using up-to-date master versions of GAP, libsemigroups, and Semigroups.

@james-d-mitchell
Copy link
Collaborator

Ok, so this also happens when the Semigroups package is not loaded at all

@james-d-mitchell
Copy link
Collaborator

james-d-mitchell commented Sep 29, 2017

So, it seems to be an issue in GAP itself, if the first group of tests is commented out.

@james-d-mitchell
Copy link
Collaborator

james-d-mitchell commented Sep 29, 2017

If the contents of the test file bug.tst is:

gap> START_TEST("bug.tst");
gap> IsAbelian("A");
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsCommutative' on 1 arguments
gap> Unbind(M);
gap> STOP_TEST("bug.tst");

and I load GAP without Semigroups (or anything else loaded using gap -A), then do

    i := 0;; repeat i := i + 1; until not Test("bug.tst");

then I get, when i is 1665

########> Diff in bug.tst:2
# Input is:
IsAbelian("A");
# Expected output:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsCommutative' on 1 arguments
# But found:
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, recursion depth trap (5000)

@james-d-mitchell
Copy link
Collaborator

james-d-mitchell commented Sep 29, 2017

Just going to check if this still holds when the Unbind(M); is removed. Yes, this occurs when i is 1665 for me without the Unbind(M); too.

@james-d-mitchell
Copy link
Collaborator

Can @wilfwilson, @flsmith and/or @mtorpey confirm?

@james-d-mitchell
Copy link
Collaborator

I'm going to file an issue

@flsmith
Copy link
Collaborator

flsmith commented Sep 29, 2017

@james-d-mitchell I get the exact same behaviour from that bug.tst

Copy link
Collaborator

@james-d-mitchell james-d-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add some tests for Embedding and Projection? Sorry forget it, I see there are plenty of tests.

@james-d-mitchell
Copy link
Collaborator

Thanks @flsmith

@wilfwilson
Copy link
Collaborator Author

@james-d-mitchell I got the same behaviour from bug.tst also.

@wilfwilson
Copy link
Collaborator Author

@james-d-mitchell Embedding and Projection are tested extensively by the helper function ProductCheck, which is defined at the top of semidp.tst and used about 30 times throughout the file.

I can add some more visible tests though.

@james-d-mitchell
Copy link
Collaborator

Sorry @wilfwilson I didn't see the tests because github collapsed the entire file. I see it is well tested! I'll have a look through, then hopefully we'll get the CI to work, then merge.

@wilfwilson
Copy link
Collaborator Author

Just saw your updated message @james-d-mitchell - I'll only add the test that I'm already working on now.

@wilfwilson
Copy link
Collaborator Author

@james-d-mitchell I'm pleased to report that Travis now passes, thanks to @ChrisJefferson's fix.

@james-d-mitchell
Copy link
Collaborator

Awesome, I'll look this over again today.

Copy link
Collaborator

@james-d-mitchell james-d-mitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good apart from the minor change to the doc.

direct product. <P/>

If these finite semigroups are either all partial perm semigroups, or all
bipartition semigroups, or all PBR semigroups, then <C>DirectProduct</C>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if they are transformation semigroups, no? Also remove "either" since there are more than two possibilities, and also remove the first "or"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that if the args are all transformation semigroups, then the result is a transformation semigroup. But that falls under the next sentence: Otherwise, <C>DirectProduct</C> returns a transformation semigroup.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right you are, I'll merge this just now

@james-d-mitchell james-d-mitchell merged commit ab0c40f into semigroups:master Oct 2, 2017
@wilfwilson wilfwilson deleted the decomposable branch October 2, 2017 14:33
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.

None yet

4 participants