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

Cayley digraphs #348

Merged
merged 3 commits into from
Aug 1, 2017
Merged

Cayley digraphs #348

merged 3 commits into from
Aug 1, 2017

Conversation

james-d-mitchell
Copy link
Collaborator

Replace Left/RightCayleyGraphSemigroup with Left/RightCayleyDigraph and introduce some functions for drawing these objects. This PR requires Digraphs version 0.10.0 which is not yet released, so the tests will probably fail.

These essentially just wrap the value previously returned by
Left/RightCayleyGraphSemigroup in a Digraph object. Methods for
Left/RightCayleyGraphSemigroup continue to exist (just calling
OutNeighbours(Left/RightCayleyDigraph) since they are defined in the
library. All other occurrences of Left/RightCayleyGraphSemigroup are
replaced with Left/RightCayleyDigraph.

@wilfwilson
Copy link
Collaborator

@james-d-mitchell This is failing because there is no code coverage of the new Dot... and Tikz... methods in display.gi.

@james-d-mitchell
Copy link
Collaborator Author

Ooops, forgot to write the doc and the tests. Done it now.

doc/fropin.xml Outdated
<C>PositionCanonical(<A>S</A>, AsListCanonical(<A>S</A>)[i] *
GeneratorsOfSemigroup(<A>S</A>)[j])</C>.
The list returned by <C>LeftCayleyGraphSemigroup</C> is defined analogously.
The list returned by <C>LeftCayleyDigraph</C> is defined analogously.<P/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sentence implies that <C>LeftCayleyDigraph</C> returns a list, rather than a digraph.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Corrected

@@ -391,8 +391,6 @@ function(S)

l := LeftCayleyGraphSemigroup(S);
r := RightCayleyGraphSemigroup(S);
# WW: in the future, when l and r are digraphs, gr can be created
# by using DigraphEdgeUnion(l, r)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason why you didn't implement this suggestion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, because there are no methods for Left/RightCayleyDigraph for semigroups not in the category IsEnumerableSemigroupRep and so we use Left/RightCayleyGraphSemigroup which are defined in the library and hence not digraphs. So, basically the comment does not make sense in this method, although it does in the method for enumerable semigroups. Hence I deleted it.

<Description>
If <A>digraph</A> is a <Ref Oper="Digraph" BookName="digraphs"/> in the
category <Ref Filt="IsCayleyDigraph" BookName="digraphs"/>, then
<C>DotString</C> returns a graphical representation of <A>digraph</A>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should be more descriptive about what the representation actually shows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We agreed this was not necessary.

James Mitchell added 3 commits July 28, 2017 11:09
These essentially just wrap the value previously returned by
Left/RightCayleyGraphSemigroup in a Digraph object. Methods for
Left/RightCayleyGraphSemigroup continue to exist (just calling
OutNeighbours(Left/RightCayleyDigraph) since they are defined in the
library. All other occurrences of Left/RightCayleyGraphSemigroup are
replaced with Left/RightCayleyDigraph.
Using dot or tikz, for digraphs and semigroups.
@james-d-mitchell
Copy link
Collaborator Author

Updated according to your comments @wi

@wilfwilson
Copy link
Collaborator

Travis is failing but it's not related to this PR. I'm trying to see if I can sort it out before merging.

@wilfwilson wilfwilson merged commit ede1124 into semigroups:master Aug 1, 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.

None yet

2 participants