Conversation
There was a problem hiding this comment.
Thanks for resubmitting. heap_down looks good and passes all the tests. I'll revise this to a green project.
🟢
Good job on the comprehension question, Kaitlyn. A key point to remember is that heaps are semi-ordered, so we can perform fewer checks when inserting. It's not a true binary search tree since it doesn't have the left/right guarantees that we have from a BST (we only know that a node is guaranteed to be bigger or smaller, depending on the style of heap, than its children). Because of the weaker ordering, maintaining the heap property in a balanced fashion is less costly than for a BST.
However, in your implementation, the remove (heap_down) is incomplete. This constitutes a significant portion of functionality missing. As such I need to evaluate this as a red submission.
Please resubmit with an implementation for heap_down.
🔴
heaps/min_heap.py
Outdated
| Time Complexity: O(log n) | ||
| Space Complexity: O(1) |
There was a problem hiding this comment.
👀 In the worst case, the new value we're inserting is the new root of the heap, meaning it would need to move up the full height of the heap (which is log n levels deep) leading to O(log n) time complexity. Your implementation of the heap_up helper is recursive, meaning that for each recursive call (up to log n of them) there is stack space being consumed. So the space complexity would also be O(log n). If heap_up were implemented iteratively, this could be reduced to O(1) space complexity.
heaps/min_heap.py
Outdated
| Time Complexity: O(log n) | ||
| Space Complexity: O(1) |
There was a problem hiding this comment.
👀 In the worst case, the value that got swapped to the top will need to move all the way back down to a leaf, meaning it would need to move down the full height of the heap (which is log n levels deep) leading to O(log n) time complexity.
You're currently missing a full implementation of the heap_down helper, but if you handled it recursively as you did for heap_up, the space complexity would also be O(log n). If heap_down were implemented iteratively, this could be reduced to O(1) space complexity.
| Space Complexity: O(1) | ||
| """ | ||
| pass | ||
| if self.empty(): |
There was a problem hiding this comment.
✨ Nice use of your own helper method!
heaps/min_heap.py
Outdated
|
|
||
| self.heap_down(0) | ||
|
|
||
| return removed_value |
There was a problem hiding this comment.
👀 We shouldn't return the internal node that we made for the heap. Instead, just return the value here. Returning the entire node was part of what was causing a test failure.
return removed_value.value| """ This method lets you print the heap, when you're testing your app. | ||
| """ | ||
| if len(self.store) == 0: | ||
| if self.empty(): |
There was a problem hiding this comment.
✨ Nice improvement to the provided str method.
| if self.empty(): | ||
| return |
There was a problem hiding this comment.
👀 This is incomplete. We would need essentially the "reverse" logic of what was done for heap_up.
heaps/heap_sort.py
Outdated
| Time Complexity: O(n * n log n) | ||
| Space Complexity: O(n) |
There was a problem hiding this comment.
👀 Since sorting using a heap reduces down to building up a heap of n items one-by-one (each taking O(log n)), then pulling them back out again (again taking O(log n) for each of n items), we end up with a time complexity of O(2n log n) → O(n log n). You have an extra n term in your time complexity.
For the space, we do need to worry about the O(log n) space consume during each add and remove (if completed), but they aren't cumulative (each is consumed only during the call to add or remove). However, the internal store for the MinHeap does grow with the size of the input list. So the maximum space would be O(n + log n) → O(n), since n is a larger term than log n.
Note that a fully in-place solution (O(1) space complexity) would require both avoiding the recursive calls, as well as working directly with the originally provided list (no internal store).
| if len(list) <= 1: | ||
| return list |
There was a problem hiding this comment.
This feels like a workaround due to not completing the heap_down method.
heaps/heap_sort.py
Outdated
| heap.add(item) | ||
|
|
||
| return_list = [] | ||
| while heap.empty() != True: |
There was a problem hiding this comment.
👀 Prefer not explicitly comparing to a boolean.
while not heap.empty():| for item in list: | ||
| heap.add(item) | ||
|
|
||
| return_list = [] |
There was a problem hiding this comment.
Your approach looks good and should work if you complete the heap_down method.
anselrognlie
left a comment
anselrognlie
left a comment
There was a problem hiding this comment.
Thanks for resubmitting. heap_down looks good and passes all the tests. I'll revise this to a green project.
🟢
|
|
||
| # I know this isn't that helpful, but it helped me make more sense of what was happening | ||
| # in the heap_down method | ||
| def is_left_index_smaller(self, index_1, index_2): |
There was a problem hiding this comment.
✨ Adding helpers to wrap your head around some noisy logic is a great strategy. It can be much nicer to read off the name of a function that literally says what it's checking rather than a complicated comparison expression and then keep reminding yourself what the intent of that comparison was.
| self.swap(index, left_child_index) | ||
| self.heap_down(left_child_index) |
There was a problem hiding this comment.
👀 Notice that each time we decide which child to swap, we swap the child, then keep heaping from that newly swapped location (following the larger value down). We could reduce the repetition somewhat by figuring out which index (if any) will be swapped, then at the end, actually do the swap and heap follow.
| @@ -1,3 +1,6 @@ | |||
| from cgitb import small | |||
There was a problem hiding this comment.
👀 Stray import (likely from VSCode autocomplete)
2 participants
Heaps Practice
Congratulations! You're submitting your assignment!
Comprehension Questions
heap_up&heap_downmethods useful? Why?