From 16264a340ab496e65a53c2864443522e612429fb Mon Sep 17 00:00:00 2001
From: Hendrik Ranocha <ranocha@users.noreply.github.com>
Date: Sun, 23 Jan 2022 14:57:34 +0100
Subject: [PATCH] BicoloredRootedTreeIterator is canonical (#73)

* BicoloredRootedTreeIterator is canonical; fixes #72

* bump patch version
---
 Project.toml         |  2 +-
 src/RootedTrees.jl   | 20 ++++++++++++++++++++
 src/colored_trees.jl | 16 ++++++++++++++--
 test/runtests.jl     | 13 ++++++++++++-
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Project.toml b/Project.toml
index d2814cbc..7a5ad031 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "RootedTrees"
 uuid = "47965b36-3f3e-11e9-0dcf-4570dfd42a8c"
 authors = ["Hendrik Ranocha <mail@ranocha.de> and contributors"]
-version = "2.9.1-pre"
+version = "2.9.1"
 
 [deps]
 Latexify = "23fbe1c1-3f47-55db-b15f-69d7ec21a316"
diff --git a/src/RootedTrees.jl b/src/RootedTrees.jl
index 218e65d9..832c70b0 100644
--- a/src/RootedTrees.jl
+++ b/src/RootedTrees.jl
@@ -446,6 +446,26 @@ function canonical_representation!(t::RootedTree{Int, Vector{Int}})
 end
 
 
+"""
+    check_canonical(t::AbstractRootedTree)
+
+Check whether `t` is in canonical representation.
+
+!!! warn "Internal interface"
+    This function is considered to be an internal implementation detail and
+    will not necessarily be stable.
+"""
+function check_canonical(t::AbstractRootedTree)
+  for subtree in SubtreeIterator(t)
+    if !check_canonical(subtree)
+      return false
+    end
+  end
+
+  return issorted(SubtreeIterator(t), rev=true)
+end
+
+
 """
     normalize_root!(t::AbstractRootedTree, root=one(eltype(t.level_sequence)))
 
diff --git a/src/colored_trees.jl b/src/colored_trees.jl
index 1049bc6f..45fa9dcb 100644
--- a/src/colored_trees.jl
+++ b/src/colored_trees.jl
@@ -395,9 +395,21 @@ end
   inner_state, color_id = state
 
   # If we can iterate more by changing the color sequence, let's do so.
-  if color_id < iter.number_of_colors
+  while color_id < iter.number_of_colors
     binary_digits!(iter.t.color_sequence, color_id)
-    return (iter.t, (inner_state, color_id + 1))
+
+    # This simple enumeration of all possible colors can also yield colored
+    # trees that are not in canonical representation. For example, the trees
+    #   rootedtree([1, 2, 2], Bool[0, 0, 1])
+    #   rootedtree([1, 2, 2], Bool[1, 0, 1])
+    # are not in canonical representation.
+    # TODO: ColoredRootedTrees. Is there a more efficient way to get only
+    #       canonical representations?
+    if check_canonical(iter.t)
+      return (iter.t, (inner_state, color_id + 1))
+    else
+      color_id = color_id + 1
+    end
   end
 
   # Now, we need to iterate to a new baseline (uncolored) tree - if possible
diff --git a/test/runtests.jl b/test/runtests.jl
index aff9361a..9115c8ba 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -792,7 +792,18 @@ end # @testset "RootedTree"
         num += 1
       end
       # number of plain rooted trees times number of possible color sequences
-      @test num == number_of_rooted_trees[order] * 2^order
+      # <= since not all possible color sequences are in canonical representation
+      @test num <= number_of_rooted_trees[order] * 2^order
+    end
+  end
+
+  # https://github.com/SciML/RootedTrees.jl/issues/72
+  @testset "BicoloredRootedTreeIterator is canonical" begin
+    for o in 1:10
+      for t_iterator in BicoloredRootedTreeIterator(o)
+        t_canonical = RootedTrees.canonical_representation(t_iterator)
+        @test t_iterator == t_canonical
+      end
     end
   end