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

Added DigraphMycielskian function #194

Merged
merged 5 commits into from
Jun 4, 2019
Merged

Conversation

MTWhyte
Copy link
Collaborator

@MTWhyte MTWhyte commented Apr 10, 2019

This pull request adds a function which takes a symmetric digraph, and returns its Mycielskian.

The Mycielskian of a symmetric digraph is a larger symmetric digraph constructed from it, which has greater chromatic number. For more information on the construction, see here.

Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

Thanks @MTWhyte, it'll be nice to have this feature. I've requested a few changes, and asked one or two questions.

doc/attr.xml Outdated
<Attr Name="DigraphMycielskian" Arg="digraph"/>
<Returns>A digraph.</Returns>
<Description>
If <A>digraph</A> is a symmetric digraph, then <C>DigraphMycielskian</C> returns the Mycielskian of <A>digraph</A>. If <A>digraph</A> is not symmetric, then an error message is returned.<P/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please delete the second sentence here? We (seem to) have adopted the convention in this package that we don't include such sentences, describing than an error is given when the argument doesn't match the requirements.

doc/attr.xml Outdated
<Description>
If <A>digraph</A> is a symmetric digraph, then <C>DigraphMycielskian</C> returns the Mycielskian of <A>digraph</A>. If <A>digraph</A> is not symmetric, then an error message is returned.<P/>

The Mycielskian of a symmetric digraph is a larger symmetric digraph constructed from it, which has a larger chromatic number. For further information, see <URL> https://en.wikipedia.org/wiki/Mycielskian </URL>. <P/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please have the <URL> tags tight against the URL, i.e.
<URL>https://en.wikipedia.org/wiki/Mycielskian</URL>

doc/attr.xml Outdated
The Mycielskian of a symmetric digraph is a larger symmetric digraph constructed from it, which has a larger chromatic number. For further information, see <URL> https://en.wikipedia.org/wiki/Mycielskian </URL>. <P/>

<Example><![CDATA[
gap> D := Digraph([[2], [1]]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you change this to CycleDigraph(2); it's the same digraph, but perhaps makes it slightly easier for a reader to understand what graph it is 🙂

gap/attr.gi Outdated
@@ -1336,6 +1336,42 @@ end);
# It is the backend to IsBipartiteDigraph, Bicomponents, and DigraphColouring
# for a 2-colouring

InstallMethod(DigraphMycielskian, "for a symmetric digraph",
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 we really need a different version of this for mutable and immutable digraphs; the one for immutable digraphs would look like this:

if not IsSymmetricDigraph(D) or IsMultiDigraph(D) then
  (blah)
fi;
return MakeImmutable(DigraphMycielskian(DigraphMutableCopy(D)));

And the mutable version would be the version that exists at the moment.

As things stand at the moment, for an immutable digraph, the current function makes a new copy in memory of the existing digraph every time you add a vertex or an edge, which happens a large number of times and harms performance.

gap> D2 := Digraph([[], [3, 4], [2, 5], [2, 6], [3, 6], [4, 5, 7], [6]]);
<immutable digraph with 7 vertices, 12 edges>
gap> IsIsomorphicDigraph(DigraphMycielskian(D1), D2);
true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also add a couple of tests of running this on a mutable digraph?

[IsDigraph],
function(D)
local n, i, j;
if not IsSymmetricDigraph(D) or IsMultiDigraph(D) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does/should this do to digraphs that have loops?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point! If we take a graph with loops like D := Digraph([[1, 2], [1]]), then DigraphMycielskian(D) has a repeated edge, which I don't want. I'll fix this.

@wilfwilson wilfwilson merged commit 8809ced into digraphs:master Jun 4, 2019
@wilfwilson
Copy link
Collaborator

Thanks for the changes @MTWhyte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature A label for new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants