[Sumtree]: Rebalancing

[crnbarr93]

Closes: #60

What is the purpose of the change

This PR is to add rebalancing logic to the cancelled order sumtree. Previously a tree could be attacked by constant insertion on one side of a tree, leading to a search runtime that trends to $O(N)$. The solution to this is to enforce post insertion rebalancing up the tree to keep it as balanced as possible.

To facilitate the rebalancing of the sumtree internal nodes must store their weight, this was added under the NodeType::Internal struct as only internal nodes need to track this data. A node's weight is determined by the depth of the node, i.e. leaf nodes have a weight of 1, their parents 2, etc. If an internal node's children have differing weights the larger of the two is taken.

        let weight = maybe_left
            .map(|n| n.get_weight())
            .unwrap_or_default()
            .max(maybe_right.map(|n| n.get_weight()).unwrap_or_default());
        self.set_weight(weight + 1)?;

Note: A leaf is given a weight of 1 when called from get_weight

To implement rebalancing a rebalance method was added to nodes, this method errors if the node is not internal or the node has no children. The exact balancing mechanism was taken from a standard AVL tree as described in #60. To facilitate balancing two rotation methods were added; rotate_right and rotate_left. These methods are called on the root of the subtree that is to be rotated, this then validates the correctness fo the node to be rotated in to its place before correctly reassigning the relationships between the nodes.

For each case in the AVL rebalancing algorithm there is an if statement, and for the LeftRight/RightLeft cases the current node (self) is resynced post child rotation using self.sync here:

        // Perform rotations based on the type of imbalance detected.
        // Case 1: Right-Right (Right rotation needed)
        if is_right_leaning && right_child_balance_factor >= 0 {
            self.rotate_left(storage)?;
        }
        // Case 2: Left-Left (Right rotation needed)
        else if is_left_leaning && left_child_balance_factor <= 0 {
            self.rotate_right(storage)?;
        }
        // Case 3: Right-Left (Right rotation on right child followed by Left rotation on self)
        else if is_right_leaning && right_child_balance_factor < 0 {
            maybe_right.unwrap().rotate_right(storage)?;
            self.sync(storage)?;
            self.rotate_left(storage)?;
        }
        // Case 4: Left-Right (Left rotation on left child followed by Right rotation on self)
        else if is_left_leaning && left_child_balance_factor > 0 {
            maybe_left.unwrap().rotate_left(storage)?;
            self.sync(storage)?;
            self.rotate_right(storage)?;
        }

Post rotation nodes have their values and ranges updated appropriate and nodes are balanced/synced as the recursive insert method unfolds.

Upon rotation if a node has no parent it is assumed to be the new root and assigned as such.

Testing and Verifying

Testing covers three main methods without using any insertion logic:

• Rotate Right
• Rotate Left
• Rebalance

All tests are contained in test_node.rs and can be run with:

cargo run unit-test --workspace

[crnbarr93]

I'm not 100% on the naming of this function but couldn't think of anything better.

It may also be unncessary if we don't use the delete method.

[crnbarr93]

Do we see any value in keeping this test?

[AlpinYukseloglu]

I think it's probably good to keep around so we can add prefix sum based assertions to it later

[AlpinYukseloglu]

LGTM, great work! Glad we finally landed on something that works :)

[AlpinYukseloglu]

Hmm what does this mean?

[crnbarr93]

I had changed the sync_range_and_value method to not iterate up the tree after syncing but this method hadn't been adjusted for that. I wrote a new method called sync_range_and_value_up that iterates up the tree and forgot to remove the comment.

[crnbarr93]

I've removed the comment now

[AlpinYukseloglu]

Is this function still necessary?

[crnbarr93]

Can't see much use for it, I'll strip it out. Easy to readd if we ever need it.

[AlpinYukseloglu]

I think it's probably good to keep around so we can add prefix sum based assertions to it later