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

Add some basic methods for MinimalSemigroupGeneratingSet #407

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

wilfwilson
Copy link
Collaborator

It is going to be very difficult to implement completely general methods for MinimalSemigroupGeneratingSet that have reasonable performance. However, I think we should still write methods that apply in special cases.

Here I've added several of those special cases. The IndecomposableElements attribute added in PR #347 was useful for this.

The biggest class of semigroups to which the new method applies is probably finite J-trivial semigroups.

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.

This looks good. I agree that it is difficult to implement this in general, and that limiting what we try is a good idea.

Would it be much work to add methods for MinimalMonoidGeneratingSet and the ones for inverse semigroups, along the lines of what you've done already? It seems like it wouldn't be. If you'd rather not do it, then please add an issue stating that it could be done in a similar way to that which you've done here.

I wonder too if it might not be worthwhile having a fallback brute force method for semigroups that are not too big?

return gens;
fi;

ErrorNoReturn("not yet implemented,");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This error message does not have the correct format.

doc/attr.xml Outdated
@@ -337,8 +337,9 @@ gap> Length(SmallGeneratingSet(S));
<Returns>A minimal generating set for a semigroup.</Returns>
<Description>

<B>Warning:</B> currently, no methods are installed to compute these
attributes.<P/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused, this documentation existed before, but there are no methods installed? Would it not make more sense to not document (or indeed declare) attributes where there are no methods installed? Maybe I misunderstood something here, but this seems like an oddity to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the example below seems to contradict the assertion that there are no methods installed for MinimalInverseMonoidGeneratingSet. The distinction between there being a method that "computes" a minimal generating set, and one that simply knows (like a monogenic inverse semigroup) a minimal generating set is probably too subtle. I suggest improving this warning to make it clearer what is going on.

@james-d-mitchell
Copy link
Collaborator

@wilfwilson any comments on my comments?

@wilfwilson
Copy link
Collaborator Author

Yes, I'll address it and hopefully make the changes at the next Wednesday meeting. A minimal monoid generating set is simply a minimal semigroup generating set, except you throw away the identity if present (except possibly for partial perms...). I'll have to think and talk to you about minimal inverse semigroup generating sets.

I'm happy to not document the attributes that genuinely have no methods installed. But it is useful to declare them so that we can set them when we create things that we know have minimal (whatever) generating sets: eg if we create a monogenic inverse monoid, for instance.

@wilfwilson
Copy link
Collaborator Author

I think I've addressed your changes. I've added methods for MinimalMonoidGeneratingSet. We decided not to do a fallback brute-force method, and I'm not immediately should what is the right thing to do for inverse semigroups.

@wilfwilson wilfwilson force-pushed the minimal-genset branch 3 times, most recently from a4dd55f to 8832ae5 Compare November 15, 2017 19:17
@wilfwilson
Copy link
Collaborator Author

The Travis tests are failing because of the coverage job, even though it shouldn't be. I think it's a problem with the profiling package. I'll investigate.

@wilfwilson
Copy link
Collaborator Author

I've changed Travis to use the master version of profiling. That seems to fix the tests. I'm happy for this to be reviewed again @james-d-mitchell and/or merged.

@james-d-mitchell
Copy link
Collaborator

@wilfwilson happy to merge this if you rebase

@wilfwilson
Copy link
Collaborator Author

Done, thanks @james-d-mitchell.

@james-d-mitchell james-d-mitchell merged commit fa3f5ab into semigroups:master Nov 21, 2017
@wilfwilson wilfwilson deleted the minimal-genset branch November 22, 2017 23:22
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.

2 participants