Improve user-facing isomorphism and canonical labelling functions, especially with colourings by wilfwilson · Pull Request #49 · digraphs/Digraphs

wilfwilson · 2017-03-10T18:44:58Z

This PR adds support for isomorphism testing, and isomorphism finding, for coloured digraphs.

Previously in the Digraphs package... you could ask for the automorphism group of a coloured digraph without multiple edges (i.e. the subgroup of the automorphism of the digraph consisting of those elements that preserve the colouring), and you could ask for the canonical labelling of a coloured digraph without multiple edges.

I've now added similar support for coloured digraphs to the operations IsIsomorphicDigraph and IsomorphismDigraphs, for testing whether two coloured digraphs without multiple edges are isomorphic as coloured digraphs (i.e. whether there exists an isomorphism between the digraphs that respects the colourings of the vertices).

In addition to these new features, I've made minor improvements to the existing code, such as checking in IsIsomorphicDigraph whether the two digraphs are equal before going on to compute a canonical labelling.

The main change of this type is a new global function DIGRAPHS_ValidateVertexPartition. Creating this function allows me to replace a large amount of duplicated code which existed to validate whether the "colouring" given as an argument to AutomorphismGroup or DigraphCanonicalLabelling was indeed a valid colouring of the relevant digraph. This is now also used by IsIsomorphicDigraph and IsomorphismDigraphs.

Given a non-negative integer n and a homogeneous list partition, this DIGRAPHS_ValidateVertexPartition tests whether partition is a valid partition [1 .. n]. A valid partition of [1 .. n] is either:

a list of length n consisting of numbers in the range [0 .. n] (so that partition[i] is the colour of vertex i), or
a list of non-empty disjoint lists whose union is [1 .. n] (so that partition[i] is the list of those vertices with colour i).

If partition is a valid partition of [1 .. n] then it returns the partition in form 1 (this is the form that bliss likes); otherwise it returns fail.

This is both a more and less permissive definition of colouring than existed previously.

It's more permissive, since in the first form, I now allow the colour 0 to be used. This is because:

bliss works fine with any colour in the range [0 .. n] (but not outside of this range), and
in the Semigroups package, a method for CanonicalBooleanMat calculates the canonical labelling of a particular colouring digraph which uses colours 0 and 1. Since colour 0 is not currently allowed in Digraphs, this method uses DIGRAPH_CANONICAL_LABELING_COLORS. I think it would be better if this method used the operation DigraphCanonicalLabelling for a digraph and a colouring.

It's less permissive, since in the second form, I now require the list to consist of disjoint lists. This was currently allowed, but to my mind that was a mistake, as it doesn't make sense for a vertex to have two colours.

In time, this global function should also replace the validation of the colouring given to HomomorphismDigraphsFinder.

However, the main work on this PR is an improvement to documentation all of the functions AutomorphismGroup, DigraphCanonicalLabelling, IsIsomorphicDigraph, and IsomorphismDigraphs in the file bliss.*.

The previous documentation for these functions was either incorrect, incomplete, or confusing (this confusion prompted issue #19). It certainly left me scratching my head for a while. I've tried to make the documentation as clear, as thorough, and as helpful as possible.

As an aside, since in the bliss.xml documentation we talk a lot about colourings, where we simply mean a labelling, I now emphasise in the documentation for DigraphColouring and ChromaticNumber that these colourings are special in some sense -- they are proper colourings.

wilfwilson · 2017-03-10T19:22:12Z

Note: this PR makes no changes to anything at the C or C++ level, nor does any of the new code directly interface with any C functions. In particular, the new methods for IsIsomorphicDigraph and IsomorphismDigraphs call the already-existing DigraphCanonicalLabelling methods. Therefore this PR is entirely independent of any changes that we may or may not be making to which software underpins this package (see the discussion on issue #40).

james-d-mitchell

This looks really good. There a couple of issues of consistency, and some more suggestions about the validate colouring function. Please make these corrections. There are only two non-minor issues:

Why don't we support multidigraphs and vertex colours? If there is a good reason for this it should be written somewhere. Maybe it is, and I overlooked it.
I don't think we should allow colour 0, this is confusing and probably was a mistake.

james-d-mitchell · 2017-03-12T16:23:18Z

  <Attr Name="ChromaticNumber" Arg="digraph"/>
  <Returns> A non-negative integer.</Returns>
  <Description>
-    A digraph coloring is a labeling of the vertices (using precisely <A>n</A>


This is weird, what is n here? This is not really related to your pull request, but since you are changing this man section anyway, why not remove the statement in brackets here? I guess this was copy pasted from another manual entry where the number of colours was specified by an argument n.

Yes this must have been copied from the documentation for DigraphColouring when the documentation for ChromaticNumber was add. I'll reword it.

james-d-mitchell · 2017-03-12T16:27:30Z

+  colours := DIGRAPHS_ValidateVertexPartition(n, list);
+  if colours = fail then
+    ErrorNoReturn("Digraphs: DigraphCanonicalLabelling: usage,\n",
+                  "the argument <list> does not define a valid colouring of ",


"Colouring" or "coloring"? There seem to be both in this file. I'm fine with either but we should probably be consistent.

I favour colouring. I'll aim for consistency (including local variable names).

james-d-mitchell · 2017-03-12T16:30:31Z

+                  "the vertices of\n<digraph>. Either <list> should be a list ",
+                  "of length <n> consisting of integers\nin the range ",
+                  "[0 .. <n>], or <list> should be a list of non-empty ",
+                  "disjoint\nlists whose union is the vertices of <digraph>,");


I think this error message could be more helpful. What is n for example? I think it would be better to add this error message to DIGRAPHS_ValidateVertexPartitionand be more specific about what is wrong (as was previously attempted). I haven't yet looked at DIGRAPHS_ValidateVertexPartition, will comment more there.

james-d-mitchell · 2017-03-12T16:31:08Z

  fi;
-
-  return DIGRAPH_CANONICAL_LABELING_COLORS(graph, colors);
+  return DIGRAPH_CANONICAL_LABELING_COLORS(digraph, colours);


Shouldn't this be `LABELLING' for consistency?

Yes, I'll make this change. The only reason I didn't change the names of the C functions is because the Semigroups package called one of these directly and I didn't want to have to fiddle with that. However I'll make a PR to Semigroups to sort this out.

james-d-mitchell · 2017-03-12T16:34:35Z

+  local m, colour1, n, colour2, class_sizes, i;
+
+  if IsMultiDigraph(gr1) or IsMultiDigraph(gr2) then
+    ErrorNoReturn("Digraphs: IsIsomorphicDigraph: usage,\n",


Why is this not supported? On the face of it, isn't this just the same as the non-coloured case?

james-d-mitchell · 2017-03-12T16:44:21Z

+    fi;
+    return fail;
+  elif IsEmpty(partition) then
+    return fail;


james-d-mitchell · 2017-03-12T16:46:03Z

-  <Oper Name="AutomorphismGroup" Label="for a digraph and a homogeneous list"
-    Arg="digraph, colors"/>
-  <Returns>A permutation group.</Returns>
+  <Returns>A permutation group, or a direct product of permutation


Isn't a direct product of perm groups a perm group?

Yes. AutomorphismGroup for a multidigraph previously had its own manual section, but I combined it with the section for a digraph without multiple edges (but without colours). The returns bit for the multidigraph section said "A direct product of permutation groups", so I just kept that bit.

Fair enough

james-d-mitchell · 2017-03-12T16:46:35Z

-    If <A>digraph</A> is a digraph without multiple edges, then this function
-    returns the automorphism group of <A>digraph</A>, as a group of
-    permutations on the vertices of <A>digraph</A>. <P/>
+    If <A>digraph</A> is a digraph, then this attribute returns the group of


returns -> stores or contains maybe?

james-d-mitchell · 2017-03-12T16:48:15Z

+        stabiliser of the vertices in the automorphism group of the edges. These
+        two groups can be accessed using the operation <Ref Oper="Projection"
+          Label="for a domain and a positive integer" BookName="ref"/>, with the
+        second argument being <C>1</C> or <C>2</C>, respectively. <P/>


This is really good, can we say which projection corresponds to which action, and actually give an example? This would be most helpful.

You probably like it because you wrote it :) I'll do what I can to improve it.

james-d-mitchell · 2017-03-12T16:49:41Z

+      A coloured digraph can be specified by its underlying digraph
+      <A>digraph</A> and its colouring <A>colours</A>. Let <C>n</C> be the
+      number of vertices of <A>digraph</A>. The colouring <A>colours</A> may
+      have one of the following two forms: <P/>


I think the <P/> here is superfluous.

wilfwilson · 2017-03-12T18:24:30Z

I will modify DIGRAPHS_ValidateVertexPartition as you describe, to display a more relevant error message, rather than returning fail and the calling function displaying a less-relevant error message.

The functions with colourings that I wrote do not support multidigraphs since the two-argument DigraphCanonicalLabelling does not support multidigraphs, it just returns fail (although this behaviour is undocumented in the released package). I'm not sure why coloured multidigraph canonical labelling was not added. I'll see if I can work out whether it is easy to add.

I'm happy to disallow the colour 0.

Another question: for a colouring of the form [[1, 4], [3], [2, 5]], we disallow empty sublists (since then it's not a homogeneous list), and this implies that the colours used have to be precisely some range [0 .. m]. However, for a colouring of the form [1, 3, 3, 1], we do not currently require that the used colours form a range, so for instance in this last example the colours used are [1, 3]. How do you think we should resolve this inconsistency? Either the first form allows empty lists, or the second doesn't allow colours to be skipped? Or we leave it as-is. ~~I suggest we leave it as it is.~~

wilfwilson · 2017-03-12T23:24:31Z

I've revisted what I had written before. For consistency's sake, I still think it's right that a colouring of the form [1, 3, 2, 3, 1] shouldn't miss out colours. I think it's more sensible this way too.

wilfwilson · 2017-03-13T01:55:22Z

I think I've sorted all of this out now.

The main change I've made since your review was to add support for passing coloured multidigraphs to bliss. This mostly involved creating a function for constructing coloured multidigraphs for bliss, and a slightly different hook function. Coloured non-multidigraphs used one trick to encode their colours and directed edges, whilst non-coloured multidigraphs used another trick to encode their multiple edges. I just combined these things together.

That means we can ask about the automorphism group and canonical labelling (and hence isomorphism) of a coloured multidigraph. It seems to work fine.

james-d-mitchell · 2017-03-14T08:10:32Z

@wilfwilson I agree about not missing colours, I think this is the only sensible option.

james-d-mitchell

Looks good, final changes:

minor change suggested to the big error in the validate colours thing,
Please declare DigraphColoring as a synonym of DigraphColouring, and any other similar methods, for backwards compatibility.
Move the validate colouring function out of utils

james-d-mitchell · 2017-03-14T08:15:50Z

    <#Include Label="EpimorphismsDigraphs">
    <#Include Label="GeneratorsOfEndomorphismMonoid">
-    <#Include Label="DigraphColoring">
+    <#Include Label="DigraphColouring">


I think we'll need to keep DigraphColoring too for backwards compatibility.

The synonym already exists in grahom.gd, and there are tests using both DigraphColoring and DigraphColouring. I just changed the name of the mansection's label.

james-d-mitchell · 2017-03-14T08:17:07Z

+  fi;
+  ErrorNoReturn("Digraphs: ", method, ": usage,\n",
+                "the argument <partition> does not define a ",
+                "colouring of the vertices [1 .. ", n, "].\nTo do so, the ",


Remove the To do so

wilfwilson · 2017-03-14T10:54:49Z

@james-d-mitchell All sorted.

wilfwilson changed the title ~~Improve user-facing bliss functions, especially with respect to colourings~~ Improve user-facing isomorphism and canonical labelling functions, especially with respect to colourings Mar 10, 2017

wilfwilson changed the title ~~Improve user-facing isomorphism and canonical labelling functions, especially with respect to colourings~~ Improve user-facing isomorphism and canonical labelling functions, especially with colourings Mar 10, 2017

wilfwilson force-pushed the improve-iso-colourings branch from 1a5aa5e to cef726f Compare March 12, 2017 11:31

james-d-mitchell requested changes Mar 12, 2017

View reviewed changes

wilfwilson force-pushed the improve-iso-colourings branch 3 times, most recently from cafc081 to 8ddc986 Compare March 13, 2017 01:47

james-d-mitchell requested changes Mar 14, 2017

View reviewed changes

wilfwilson added 6 commits March 14, 2017 10:32

bliss: introduce DIGRAPHS_ValidateVertexPartition

f2e5e57

doc: clarify that colourings are proper, as needed

6b2f460

grahom: use colouring validator and rename for consistency

fe34cea

src: renaming for consistency

4a47a82

src: automorphisms/canonical labelling for coloured multidigraphs

6cc81e8

bliss: clarify and extend functionality and documentation

97c9fae

wilfwilson force-pushed the improve-iso-colourings branch from 8ddc986 to 97c9fae Compare March 14, 2017 10:41

james-d-mitchell merged commit 50145cf into digraphs:stable-0.6 Mar 14, 2017

wilfwilson deleted the improve-iso-colourings branch March 14, 2017 11:36

wilfwilson mentioned this pull request Mar 14, 2017

Digraph canonical labelling not respecting colours #19

Closed

Conversation

wilfwilson commented Mar 10, 2017

Uh oh!

wilfwilson commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

james-d-mitchell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilfwilson commented Mar 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilfwilson commented Mar 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilfwilson commented Mar 13, 2017

Uh oh!

james-d-mitchell commented Mar 14, 2017

Uh oh!

james-d-mitchell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilfwilson commented Mar 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wilfwilson commented Mar 10, 2017 •

edited

Loading

wilfwilson commented Mar 12, 2017 •

edited

Loading

wilfwilson commented Mar 12, 2017 •

edited

Loading