Skip to content

fix(node): re-balance new tip after srotate promotees - #157

Open
IMGillusion wants to merge 1 commit into
chaimleib:masterfrom
IMGillusion:fix/135/unbalanced-bulk-construction
Open

IMGillusion wants to merge 1 commit into
chaimleib:masterfrom
IMGillusion:fix/135/unbalanced-bulk-construction

Conversation

@IMGillusion

Copy link
Copy Markdown

Building a tree from a bulk list (IntervalTree(ivs) /
from_tuples) could leave the root with a balance of +-2,
failing verify():

from intervaltree import IntervalTree
intervals = [[478019, 486075], [478041, 483497], [478215, 486619],
             [478416, 488227], [479107, 483366], [479112, 488300],
             [479867, 489663], [481266, 490406], [482870, 485066],
             [483050, 487543], [484692, 485957], [488408, 489859],
             [490057, 492197], [491277, 493556]]
IntervalTree.from_tuples(intervals).verify()
# AssertionError: Error: Rotation should have happened, but
# didn't!

Root cause -- srotate() ends with save.refresh_balance().
The promotees step that runs just before it moves intervals out of
the light child and, as a side effect of remove(), can prune that
child entirely. Once the light child is gone the new tip (save) is
left with a balance of +-2, but it is only refreshed, never
re-balanced.

Fix -- re-balance the returned tip with save.rotate() instead of
save.refresh_balance(). rotate() is a no-op when the tip is already
balanced (the common case), and corrects the balance when the promotees
step disturbed it. One-line change.

Verification

  • new regression test test/issues/issue135_test.py (RED before, GREEN after)
  • full suite: 104 passed, 0 regressions
  • 3000-trial random fuzz (1-40 intervals, bulk + sequential + add/remove):
    verify() always passes, no data loss

Fixes #135

Bulk construction (IntervalTree(ivs) / from_tuples) could leave the
root with a balance of +-2 because srotate() only refreshed the
balance of the new tip after its promotees step. When that step
promotes enough intervals out of the light child to prune it
entirely, the tip is left unbalanced and verify() fails:

    AssertionError: Error: Rotation should have happened, but
    didn't!

Re-balance the returned tip instead of merely refreshing it. This
is a no-op when the tip is already balanced, and corrects the
balance when the promotees step disturbed it.

Fixes chaimleib#135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

specific intervals cause inbalanced nodes

1 participant