Skip to content
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

Lilly C16 Pine #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

waterlilly169
Copy link

Heaps Practice

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
How is a Heap different from a Binary Search Tree? Insertion times are generally better
Could you build a heap with linked nodes? yes
Why is adding a node to a heap an O(log n) operation? This is O(log n) because moving the node down the heap can potentially require O(log n) swaps.
Were the heap_up & heap_down methods useful? Why? Yes, they made insertion easier

@anselrognlie anselrognlie self-requested a review July 12, 2022 22:11
Copy link

@anselrognlie anselrognlie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨💫 Nice job, Lilly. I left some comments on your implementation below.

In response to your comprehension questions, the reason that insertion times are generally better comes from the looser guarantees about the ordering. Heaps are semi-ordered, so we can perform fewer checks when inserting. And while a heap can be implemented using a linked list it's generally not the ideal structure. We prefer arrays since we can easily access parents and children using their index.

🟢

"""
pass
if value == None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 Prefer using is to compare to None

Comment on lines +22 to +23
Time Complexity: O(log n)
Space Complexity: O(1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Great! 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). Your implementation of the heap_up helper is iterative, meaning that while we do have to loop log n times, we don't have any recursive stack overhead. So the space complexity is constant (O(1)).

"""
pass
if len(self.store) == 0:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 We have an empty helper we could use here

Comment on lines +35 to +36
Time Complexity: O(log n)
Space Complexity: O(1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Nice. Just as for add, the complexities are really coming from the heap_down implementation. And like your heap_up, by taking an iterative approach, you achieve O(1) space complexity.

Comment on lines +61 to +62
Time complexity: O(1)
Space complexity: O(1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.



# method to move heap down
def heap_down(self, index):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Nice approach of first determining the candidate child, then deciding whether the swap is required.

Though not prompted, like heap_up, heap_down is also O(log n) in time and constant in space complexity. The worst case for re-heapifying is if the new root need to move back down to a leaf, requiring log n iterations. And since you took the iterative approach, there's no additional stack growth, leaving us with constant space.

Comment on lines +101 to +104
if temporary == index:
break
self.swap(temporary, index)
index = temporary

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ This mirrors my suggestion for how to rewrite the condition in heap_up. Notice how we get to unindent the main logic here as well!

largest = r
if largest != i:
arr[i], arr[largest] = arr[largest], arr[i]
heapify(arr, n, largest)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider reworking this to be iterative. As written, this is basically a max heap heap_down (pushes down smaller numbers). The addition of the n (size) parameter lets you arbitrarily set what the "end" of the heap is, allowing the data array to be segmented into a heap portion, and a non-heap (becomes the sorted) portion. But since this is written recursively, we can't so better than O(log n) for time and space for any given call (the log n space comes from recursive call stacks).

If implemented iteratively, this would have time complexity O(log n), but space complexity O(1).

Alternatively, it's not too much work to start from the MinHeap implementation, but remove the need for the HeapNode, and initialize the store to use the list to be sorted directly. Switching to a max heap is a great idea (to sort ascending), and only a small change would be required to switch from the min heap to a max heap.

Comment on lines +16 to +17
Time Complexity: O(n)
Space Complexity: O(n)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 Since each call to your heapify helper takes O(log n) time, and we are calling it 2n times here, the time complexity of heap_sort will be O(2n log n) → O(n log n). Note that for a general sorting algorithm (like heap sort), O(n log n) is the theoretical best we can do!

For space complexity, you avoided needing to allocate the internal store that MinHeap used, but since heapify is implemented recursively, the stack will grow with the number of layers in the heap. So each heapify call will have a stack with O(log n) levels. Each is used only for the duration of a single heapify call, so they aren't cumulative, resulting in an overall space complexity of O(log n). But if you modified your heapify to be iterative, the space complexity would be constant O(1)!

Comment on lines +20 to +24
heapify(arr, n, i)
for i in range(n - 1, 0, -1):
arr[i], arr[0] = arr[0], arr[i]
heapify(arr, i, 0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice approach of building a max heap in place (though with the additional stack cost), and then moving the max value in the heap to end end of the list, heapifying the remaining data.

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.

2 participants