-
Notifications
You must be signed in to change notification settings - Fork 9
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
Segment tree enhancements #56
Segment tree enhancements #56
Conversation
The reference code for the segment tree can now store different types of data, depending on what is specified as the type in the SegmentTreeType variable 'type'. Using the helper function in segment tree operations automatically handles returning the appropriate data to store in the nodes of the tree.
In terms of contest problems, I get quite a few hits when searching Google for “segment tree site:usaco.org”. “To properly represent tree” -> “To properly represent the tree” Maybe in the initial paragraph make it clear that prefix sum precalculation works for range sums but not range mins or maxes. I like the new figure, but I find it confusing reconciling it with figure 1.1, and in particular I find the “empty spaces” in the leaves of the segment tree a bit confusing when the bottom of the tree is mapped to a contiguous array as in the example. Normally, if the leaves of the segment tree map to an array, the leaves would be completely filled in by the contents of the array in a contiguous fashion instead of leaving holes like this. I think the problem is easier to read now. Maybe someone else wants to have a look as well though just to make sure it’s clear on first impression (since now I know what the problem is supposed to be asking). Best, From: Marshall Clyburn [mailto:[email protected]] The reference code for the segment tree can now easily support other operations. I've manually tested all three implementations that are there. I'm also looking into writing a test for all three that are there at the same time. I only have to figure how that would work. The code here is up for review. Any feedback to make this more readable is more than welcome. I didn't make the SegmentTreeType an argument to the functions because there is already a sizable amount of arguments to at least one of the functions. I can see that this may be a problem if two segment trees of different types are needed though. This is changeable, but I would like to know if this should perhaps be the default? This is also for another round of reviewing the sample problem included with the section. I aimed for a clear input format that has no extra twists to it. This makes it simpler to understand as well. Pinging @bcdeanhttps://github.com/bcdean, @robertu94https://github.com/robertu94, and perhaps even @ProtractorNinjahttps://github.com/ProtractorNinja since I don't recall you ever seeing the sample problem. Also, does anyone know of a real contest problem that called for a segment tree? That would be nice to include in this section as well. You can view, comment on, or merge this pull request online at: Commit Summary
File Changes
Patch Links:
— |
@bcdean Do you think that it would be better that I left out the empty nodes so that the leaves are the exact same as the array? I avoided using a full array to better show the flexibility of the data structure, but maybe that's not necessary? Thanks for the suggestions. |
Probably best to do this. I agree it’s nice to show the flexibility of the structure, but if this were to be done, you might want to use a different example than one where the leaves are considered to be mapped to an array. -- Brian From: Marshall Clyburn [mailto:[email protected]] @bcdeanhttps://github.com/bcdean Do you think that it would be better that I left out the empty nodes so that the leaves are the exact same as the array? I avoided using a full array to better show the flexibility of the data structure, but maybe that's not necessary? Thanks for the suggestions. — |
void update_segment_tree(int* const tree, const unsigned int start, const unsigned int end, const unsigned int changed_idx, const int new_val, const unsigned int st_idx) | ||
{ | ||
// #ifdef hackpackpp | ||
// update a value in the segment tree | ||
// | ||
// To support range sums, this function would have to be modified to recurse |
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 isn't strictly relevant to the solving of the problem. Instead of this long comment here, just include segment-tree.cpp
in a section called "Reference" and put this comment there.
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'm removing it. I didn't realize that this was irrelevant when I was going through it. Since the reference code easily does this now, it shouldn't be here any more.
As for including the type of the segment tree, I would group the functions into a You could even include it as a default argument in c++98 which we see at ACM regional for whatever reason like so class SegmentTree{
public:
unsigned int N;
int * data;
SegmentTree(int size,SegmentTreeType tree_type=ST_MIN){/* etc */}
/* etc */
}; |
I still think the input format section could be a tad clearer that each of the C blocks refers to a specific cow, for example: % ...
\item The following lines are placed in $C$ groups detailed as follows:
\begin{itemize}
\item A single line containing a single integer, $k$, representing the number of time ranges to follow that a single cow was present in the barn.
\item $k$ lines with two distinct integers representing times between which that cow was present in the barn.
The first number is guaranteed to be strictly less than the second number.
\item A single line containing two integers $t_1$ and $t_2$ representing times between which the maximum number of cows that have been entered so far that could seen in the barn simultaneously
%... |
The hint on changing the function to support range sums is no longer important since this is easily done by the segment tree provided in the reference code.
The class version is a bit more heavy-weight than the set of functions, but this should support multiple segment trees of varying types at once.
Anyone else have feedback for this? If not, I'd like to get this merged in. |
The problem looks good at this point; however, the reference code should also be in the hackpack++ version. After that is fixed, it looks good to me. |
Oh. Good. Yes. I was just thinking about looking at this. Let this be a lesson to me. |
The reference code for the segment tree can now easily support other operations. I've manually tested all three implementations that are there. I'm also looking into writing a test for all three that are there at the same time. I only have to figure how that would work. The code here is up for review. Any feedback to make this more readable is more than welcome. I didn't make the SegmentTreeType an argument to the functions because there is already a sizable amount of arguments to at least one of the functions. I can see that this may be a problem if two segment trees of different types are needed though. This is changeable, but I would like to know if this should perhaps be the default?
This is also for another round of reviewing the sample problem included with the section. I aimed for a clear input format that has no extra twists to it. This makes it simpler to understand as well. Pinging @bcdean, @robertu94, and perhaps even @ProtractorNinja since I don't recall you ever seeing the sample problem.
Also, does anyone know of a real contest problem that called for a segment tree? That would be nice to include in this section as well.
Takes care of issue #45.