Skip to content

Commit 2e2d1cf

Browse files
author
Release Manager
committed
gh-39945: Fixing high complexity canonical_labelling for OrderedTree and BinaryTree Added canonical_labelling overwrite to OrderedTree, modified the same method in BinaryTree, so that the complexity is linear, instead of quadratic in the original implementation. This solves the issue #35930, which arises when using Sagemath to do linear-time random generation of large tree-like structures based on OrderedTree or BinaryTree when a canonical labelling is needed, in which case the quadratic nature of the original algorithm slows down a linear random generator. This is done by using a "boxed" value to keep track of already-assigned labels, instead of computing the number of nodes at each time for the shifting. The precise implementation takes care so that the canonical labellings given in the examples are not changed. Comments are also updated. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies #35930: This PR solves the given issue. URL: #39945 Reported by: fwjmath Reviewer(s): fwjmath, Travis Scrimshaw
2 parents b0a287c + e9385c0 commit 2e2d1cf

File tree

2 files changed

+91
-27
lines changed

2 files changed

+91
-27
lines changed

src/sage/combinat/abstract_tree.py

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,14 +1198,28 @@ def node_number(self):
11981198
sage: T.node_number()
11991199
10000
12001200
"""
1201-
count = 0
1201+
# An attribute _node_number for storing the number of nodes
1202+
# This is OK as AbstractTree is immutable
1203+
# If the attribute is not present, we compute it
1204+
try:
1205+
return self._node_number
1206+
except AttributeError:
1207+
pass
1208+
1209+
if self.is_empty():
1210+
self._node_number = Integer(0)
1211+
return self._node_number
12021212

1203-
def incr(node):
1204-
nonlocal count
1205-
count += 1
1213+
def count(node):
1214+
if not node.is_empty():
1215+
# Using post-order
1216+
# Thus _node_number is computed for all non-empty subtrees
1217+
node._node_number = Integer(1)
1218+
node._node_number += sum(e._node_number for e in node
1219+
if not e.is_empty())
12061220

1207-
self.iterative_pre_order_traversal(incr)
1208-
return Integer(count)
1221+
self.iterative_post_order_traversal(count)
1222+
return self._node_number
12091223

12101224
def depth(self):
12111225
"""
@@ -1518,13 +1532,14 @@ def canonical_labelling(self, shift=1):
15181532
sage: BinaryTree().canonical_labelling()
15191533
.
15201534
"""
1521-
LTR = self.parent().labelled_trees()
1522-
liste = []
1523-
deca = 1
1524-
for subtree in self:
1525-
liste += [subtree.canonical_labelling(shift + deca)]
1526-
deca += subtree.node_number()
1527-
return LTR._element_constructor_(liste, label=shift)
1535+
def aux(tree, LTR, curlabel):
1536+
mylabel = curlabel[0]
1537+
curlabel[0] += 1
1538+
newtree = LTR([aux(st, LTR, curlabel) for st in tree],
1539+
label=mylabel)
1540+
return newtree
1541+
1542+
return aux(self, self.parent().labelled_trees(), [shift])
15281543

15291544
def to_hexacode(self):
15301545
r"""
@@ -2181,6 +2196,41 @@ def __getitem__(self, idx):
21812196
else:
21822197
return ClonableArray._getitem(self, i)
21832198

2199+
def _require_mutable(self):
2200+
"""
2201+
Check that ``self`` is mutable and clear the ``_node_number`` parameter.
2202+
2203+
The mutation could affect a precomputed ``_node_number``,
2204+
and so we clear this when this is called, which is done in
2205+
preparation for mutating the tree.
2206+
2207+
Values cleared:
2208+
2209+
- ``_node_number`` : number of nodes in the tree
2210+
2211+
EXAMPLES::
2212+
2213+
sage: T = OrderedTree()
2214+
sage: T._require_mutable()
2215+
Traceback (most recent call last):
2216+
...
2217+
ValueError: object is immutable; please change a copy instead.
2218+
2219+
::
2220+
2221+
sage: T = LabelledRootedTree([]).clone()
2222+
sage: T.node_number()
2223+
1
2224+
sage: T._require_mutable()
2225+
sage: hasattr(T, '_node_number')
2226+
False
2227+
"""
2228+
super()._require_mutable()
2229+
try:
2230+
delattr(self, '_node_number')
2231+
except AttributeError:
2232+
pass
2233+
21842234

21852235
class AbstractLabelledTree(AbstractTree):
21862236
"""

src/sage/combinat/binary_tree.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -822,12 +822,23 @@ def canonical_labelling(self, shift=1):
822822
r"""
823823
Return a labelled version of ``self``.
824824
825-
The canonical labelling of a binary tree is a certain labelling of the
826-
nodes (not the leaves) of the tree.
827-
The actual canonical labelling is currently unspecified. However, it
828-
is guaranteed to have labels in `1...n` where `n` is the number of
829-
nodes of the tree. Moreover, two (unlabelled) trees compare as equal if
830-
and only if their canonical labelled trees compare as equal.
825+
The actual canonical labelling is currently unspecified. However,
826+
currently we use the labelling of the internal nodes (not the leaves)
827+
of the tree given by the in-order, i.e., the order defined recursively
828+
using the rule that, for each node `u`, its descendants in the left
829+
(resp. right) subtree comes before (resp. after) `u`.
830+
831+
Under any future changes, we guarantee that the labels by default is
832+
from `1` to `n`, with `n` the number of nodes of the tree. The smallest
833+
labeling is adjustable using the parameter ``shift``. Moreover, we
834+
guarantee that two (unlabelled) trees compare as equal if and only if
835+
their canonical labelled trees compare as equal.
836+
837+
The runtime of this method is linear in the number of nodes.
838+
839+
INPUT:
840+
841+
- ``shift`` -- integer (optional); the smallest label in the tree labelling
831842
832843
EXAMPLES::
833844
@@ -838,14 +849,17 @@ def canonical_labelling(self, shift=1):
838849
sage: BinaryTree([[[], [[], None]], [[], []]]).canonical_labelling()
839850
5[2[1[., .], 4[3[., .], .]], 7[6[., .], 8[., .]]]
840851
"""
841-
LTR = self.parent().labelled_trees()
842-
if self:
843-
sz0 = self[0].node_number()
844-
return LTR([self[0].canonical_labelling(shift),
845-
self[1].canonical_labelling(shift + 1 + sz0)],
846-
label=shift + sz0)
847-
else:
848-
return LTR(None)
852+
def aux(tree, LTR, curlabel):
853+
if not tree:
854+
return LTR(None)
855+
ltree = aux(tree[0], LTR, curlabel)
856+
mylabel = curlabel[0]
857+
curlabel[0] += 1
858+
rtree = aux(tree[1], LTR, curlabel)
859+
newtree = LTR([ltree, rtree], label=mylabel)
860+
return newtree
861+
862+
return aux(self, self.parent().labelled_trees(), [shift])
849863

850864
def show(self, with_leaves=False):
851865
"""

0 commit comments

Comments
 (0)