Update OrdPSQ balance logic with results from Milan Straka's research. Fixes #63 #64
Conversation
This change tightens the test condition to exactly match the balance property described in the Hinze and Adams papers. This causes the Quickcheck test for tree validity to fail in most runs. This is at least somewhat expected, as it has been established that Adams' balancing algorithm fails in some edge cases, and Hinze's paper was written before that discovery.
Milan Straka's paper 'Adams’ Trees Revisited' (currently available at http://fox.ucw.cz/papers/bbtree/bbtree.pdf ) argues that Adams' balancing algorithm previously did not have a correct proof. It provides a proof of a slightly modified balancing algorithm that prefers single rotations in many more cases. The small change specified in that paper fixes the frequent quickcheck failures exposed when the balance test was made more precise.
Several added tests duplicate functionality testing between the LTree and TourView interpretations. The duplication is intentional, to help ensure conversion between those interpretations is correct.
|
@jaspervdj Is there anything I need to do to kick the CI runners? |
|
Hi @chowells79, thanks once more for the very impressive work! I am returning from holiday this today and should be able to review it this week. |
|
I restarted the CI but it seems like they just get stuck in a queueing state, I'm not sure what's going on. I'll take a look at that as well when I review the changes. |
In a hilarious bit of bad timing, the link I was using last week no longer works now.
|
In some hilarious bad timing, Milan Straka's paper is no longer available at the URL I used above and in the documentation. I've updated the documentation to use a link that still works and added the name/author of the paper so that it can be found when the current link eventually rots. For completeness, the new link is https://ufal.mff.cuni.cz/~straka/papers/2011-bbtree.pdf |
|
@jaspervdj just a ping. This isn't high priority work - the existing code is probably good enough to never break down in a way that breaks performance badly. But I'd still like to make sure it doesn't get lost after all the trouble I went to tracking down explanations of what was wrong in Hinze's paper. |
* Tighten OrdPSQ balance property test. This change tightens the test condition to exactly match the balance property described in the Hinze and Adams papers. This causes the Quickcheck test for tree validity to fail in most runs. This is at least somewhat expected, as it has been established that Adams' balancing algorithm fails in some edge cases, and Hinze's paper was written before that discovery. * Use Milan Straka's updated balancing algorithm for OrdPSQ Milan Straka's paper 'Adams’ Trees Revisited' (currently available at http://fox.ucw.cz/papers/bbtree/bbtree.pdf ) argues that Adams' balancing algorithm previously did not have a correct proof. It provides a proof of a slightly modified balancing algorithm that prefers single rotations in many more cases. The small change specified in that paper fixes the frequent quickcheck failures exposed when the balance test was made more precise. * Additional tests for OrdPSQ's internal invariants Several added tests duplicate functionality testing between the LTree and TourView interpretations. The duplication is intentional, to help ensure conversion between those interpretations is correct. * Update link to Straka's paper. In a hilarious bit of bad timing, the link I was using last week no longer works now.
2 participants
Milan Straka's Adams’ Trees Revisited makes a small change to the tree balancing logic proposed in Adams' paper and copied into Hinze's. After updating the balance condition test to be exactly what the invariants said, the test suite frequently found cases where the balance invariants were being violated. Updating with Straka's logic fixed all those cases.
I also added a number of tests to ensure that none of the internal invariants the LTree operations depend on are violated.