-
Notifications
You must be signed in to change notification settings - Fork 47
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
Conversation
There was a problem hiding this 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/> |
There was a problem hiding this comment.
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/> |
There was a problem hiding this comment.
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]]); |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Thanks for the changes @MTWhyte |
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.