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 a second symmetric group presentation due to Moore #431

Merged

Conversation

MTWhyte
Copy link
Collaborator

@MTWhyte MTWhyte commented Dec 12, 2022

This PR adds a second symmetric group presentation due to Moore, and therefore sets up the symmetric_group function to receive a third argument called index which can be used to choose between presentations of the same author.

In a separate commit, default value tests for the function cyclic_inverse_monoid are added.

@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Dec 12, 2022

@james-d-mitchell a particular question I have is: do you think the way I've documented index is clear enough? I've tried to get across that an exception will be thrown if index is out of bounds, without writing a new\throws LibsemigroupsException specific to every single choice of author.

@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Dec 12, 2022

I also considered adding the numbers of generators and relations for the symmetric group presentations while I was doing this. Since there are quite a lot of presentations (more than for cyclic_inverse_monoid and order_preserving_monoid), I wonder whether instead of having the list of bullet points with citations in brackets, we should have a table with fields author, index, reference, number of generators and number of relations?

@james-d-mitchell james-d-mitchell added the new-feature Label for issues or PR about new features label Dec 13, 2022
@james-d-mitchell
Copy link
Member

@james-d-mitchell a particular question I have is: do you think the way I've documented index is clear enough? I've tried to get across that an exception will be thrown if index is out of bounds, without writing a new\throws LibsemigroupsException specific to every single choice of author.

I think what you've got is clear, maybe you could combine the first and third throws and just say that it throws if the author index combination is invalid?

@james-d-mitchell
Copy link
Member

I wonder whether instead of having the list of bullet points with citations in brackets, we should have a table with fields author, index, reference, number of generators and number of relations?

That'd be great, good idea.

@james-d-mitchell
Copy link
Member

Also @MTWhyte would you mind rebasing this onto main, I made some changes to the CI. Thanks

@MTWhyte MTWhyte force-pushed the second-moore-Sn-presentation branch 3 times, most recently from 1df4ca9 to 146aead Compare December 14, 2022 16:23
@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Dec 14, 2022

I've got the table working properly now. Including NOLINT as an html comment seems to have worked. A problem worth noting for future reference is, it appears that if the code for the table contains a line longer than about 160 characters, the table doesn't build properly.

@james-d-mitchell
Copy link
Member

Thanks @MTWhyte

@james-d-mitchell james-d-mitchell merged commit b3173d9 into libsemigroups:main Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature Label for issues or PR about new features
Development

Successfully merging this pull request may close these issues.

2 participants