Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Link Cut Tree #124
base: main
Are you sure you want to change the base?
Link Cut Tree #124
Changes from 7 commits
e6aa29b
2d00e3b
805cc15
1a03c46
7887f5e
1c30800
93f1be2
06b3bd5
5dc6f51
0216a13
6fbfd26
673c2cc
613a162
1fdda13
e95227d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to remove the old link-cut tree, so feel free to do that and modify the fuzz-test. Will also need to update the test to verify that max works (with max replaced by some non-commutative, non-associative function)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the fuzz-test to verify max for now. Will update it later for any new function that we decide on...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to add a one-line comment explaining what p and pp are? or are they standard names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p = parent, pp = path-parent (and c = children).
p and c are straight forward. path-parent is from the Wikipedia article on LCTs and also this set of lecture notes https://courses.csail.mit.edu/6.851/spring12/scribe/L19.pdf (which I think the Wikipedia page is based on). Besides writing out the acronyms, do you think it's a good idea to explain what a path-parent is? I think that if you understand how LCTs work (by decomposing a tree into preferred paths), then knowing that pp stands for path-pointer should be enough information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So something like
// p ~ parent in segtree, pp = path parent
? (Now, I'm also not sure whether that's helpful information when augmenting... but I could imagine such a comment being helpful for understanding)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this comment as well -- I think we can instead write in the description how to augment it ("extend push/pull").
We'll also want a more thorough explanation and/or example -- it's not clear to me in which ways the LCT is similar to a segment tree. E.g. how do I do lazy updates on entire paths in the tree? Entire subtrees? Queries on subtrees? Can I use the flip bit myself for anything? How are things ordered? It's not reasonable to answer all of these questions (we should have a link in the description!), but some more info would be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work with non-commutative and non-associative operations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've talked about this elsewhere, but it should work for non-commutative, associative functions (like a segment tree).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an unreadable mess, it wouldn't hurt to golf it a bit and make it even more unreadable... some ideas:
, *&w = y->c[!t]
at the top and usew
instead ofc[!t]
y->p = this
andp = z
could go together?Also, I'm a bit surprised the pull for z happens before that of y, I would have expected the other way around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be misunderstanding, but why *&w instead of *w?
Also y->c[!t] is only used once, while this->c[!t] is used multiple times, so *&w = c[!t] might be better.
Like the fix function in the previous lct? (Set a node's children, and then fix will set those children's parents, but without the "(+ update sum of subtree elements etc. if wanted)" part, since that is what pull does).
Pull for z was unnecessary. (A node is only moved up one level for each call of rot, and then pulls up itself at the end of splay.) Thanks for noticing.
Everything else seems good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I confused
y->c[!t]
andc[!t]
.c[!t]
is assigned to, hence the&
.Yeah. I don't know much about how the previous lct works, but it seems like a method along those lines may be able to cut away a few lines? Again, I'm just throwing ideas out, I don't know whether it actually does save anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like adding fix will shorten it since
w->p = y;
is already as short asw->fix();
. And then sometimes we'll have to set a parent directly anyway. (whenz = 0
and we need to setthis->p = z
then we need to setp = z
without callingz->fix()
anyway). I think it's ok to leave as it is.The other changes have been put in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should probably rename this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"propogatePathParent" is a little much though... Will keep thinking about possible alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could certainly shorten that to
propPP
if that's what we want to get across. But a nonsensical name would be fine as wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this function on one line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, but goes slightly over the line limit now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, that's no good then... Either revert or change the function names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted