SQUASHME some updates.

Author: jeromekelleher

python/tests/test_topology.py

@@ -7954,6 +7954,15 @@ def test_star_branch_length(self):
         assert ts.kc_distance(topological_equiv_ts) == 0
 
 
+def num_leaf_labelled_binary_trees(n):
+    """
+    Returns the number of leaf labelled binary trees with n leaves.
+
+    https://oeis.org/A005373/
+    """
+    return int(np.math.factorial(2 * n - 3) / (2 ** (n - 2) * np.math.factorial(n - 2)))
+
+
 class TestPolytomySplitting:
     """
     Test the ability to randomly split polytomies
@@ -7999,15 +8008,17 @@ def ts_polytomy_44344(self):
             strict=False,
         )
 
-    def test_all_topologies_n4(self):
+    @pytest.mark.parametrize("n", [2, 3, 4, 5])
+    def test_all_topologies(self, n):
+        N = num_leaf_labelled_binary_trees(n)
         ranks = collections.Counter()
-        t4 = self.tree_polytomy_4()
-        for _ in range(150):
-            tree = t4.split_polytomies()
-            ranks[tree.rank()] += 1
-        # There are 15 possible binary trees here, we should have seen them
-        # all with high probability after 150 attempts.
-        assert len(ranks) == 15
+        tree = tskit.Tree.generate_star(n)
+        for seed in range(20 * N):
+            split_tree = tree.split_polytomies(random_seed=seed)
+            ranks[split_tree.rank()] += 1
+        # There are N possible binary trees here, we should have seen them
+        # all with high probability after 20 N attempts.
+        assert len(ranks) == N
 
     def test_simple_examples(self):
         tree = self.tree_polytomy_4().split_polytomies(random_seed=12)
@@ -8084,11 +8095,12 @@ def test_epsilon_for_mutations(self):
             site=site, time=root_time - mut_diff, node=0, derived_state="1"
         )
         tree = tables.tree_sequence().first()
+        # FIXME this test is brittle and fails for some seeds
         with pytest.raises(
             _tskit.LibraryError,
             match="not small enough to create new nodes below a polytomy",
         ):
-            tree.split_polytomies(epsilon=mut_diff * 2, random_seed=13)
+            tree.split_polytomies(epsilon=mut_diff * 2, random_seed=123)
         # A 4-tomy creates 2 new nodes => epsilon of ~ 1/3 of mut_diff should work
         self.tree_polytomy_4().split_polytomies(epsilon=mut_diff / 3, random_seed=13)
 

python/tskit/combinatorics.py

@@ -55,7 +55,7 @@ def random_binary_tree(leaf_labels, rng):
         implementation of Algorithm R, because we are interested in
         the leaf node labellings.
 
-        The pre-fasicle text is available here, page 16:
+        The pre-fascicle text is available here, page 16:
         http://www.cs.utsa.edu/~wagner/knuth/fasc4a.pdf
         """
         nodes = [TreeNode(label=leaf_labels[0])]
@@ -66,6 +66,7 @@ def random_binary_tree(leaf_labels, rng):
             if x.parent is not None:
                 index = x.parent.children.index(x)
                 x.parent.children[index] = internal
+            # TODO it's not clear if this shuffle is actually needed.
             rng.shuffle(internal.children)
             x.parent = internal
             n.parent = internal

python/tskit/trees.py

@@ -2387,11 +2387,9 @@ def split_polytomies(
 
         If the ``method`` is ``"random"`` (currently the only option, and the default
         when no method is specified), then for a node with :math:`n` children, the
-        :math:`(2n - 3)!!` possible bifurcating topologies are produced with equal
+        :math:`(2n - 3)! / (2^(n - 2) (n - 2!))` possible binary trees with equal
         probability.
 
-        .. fixme:: Looks like this function is wrong
-
         The returned :class`.Tree` will have the same genomic span as this tree,
         and node IDs will be converved (that is, node ``u`` in this tree will
         also exist in the returned tree). The returned tree is derived from a