2

I'm currently doing an assignment for my computer science class, and I can't figure out what is wrong with my code to perform quick select.

def partition(a_list, first, last):
    pivot = a_list[last]
    i = first-1
    for j in range(first, last):
        if a_list[j] <= pivot:
            i += 1
            a_list[i], a_list[j] = a_list[j], a_list[i]
    a_list[i+1], a_list[last] = a_list[last], a_list[i+1]
    print(a_list)
    return i+1

def selection(a_list, first, last, k):
    pivot = a_list[last]
    pivotIndex = partition(a_list, first, last)
    if first == last:
        return a_list[k]
    elif k <= pivotIndex:
        return selection(a_list, first, pivotIndex-1, k)
    else:
        return selection(a_list, pivotIndex+1, last, k - pivotIndex)

print(selection([5,4,1,10,8,3,2], 0, 6, 1))
print(selection([5,4,1,10,8,3,2], 0, 6, 3))
print(selection([5,4,1,10,8,3,2], 0, 6, 6))
print(selection([5,4,1,10,8,3,2], 0, 6, 7))
print(selection([46, 50, 16, 88, 79, 77, 17, 2, 43, 13, 86, 12, 68, 33, 81, \
74, 19, 52, 98, 70, 61, 71, 93, 5, 55], 0, 24, 19))

After the third print statement my code just get stuck in a loop, and eventually dies because maximum recursions have been reached. The first output should also be 1, and I understand why it's doing that. But I can't figure out a solution to fix it.

This is my output, before it eventually gives me the error maximum recursion depths reached. (Ignore the list being printed, it's just there so I can see what it's partitioning)

[1, 2, 5, 10, 8, 3, 4]
2
[1, 2, 5, 10, 8, 3, 4]
[1, 2, 3, 4, 8, 5, 10]
3
[1, 2, 5, 10, 8, 3, 4]
[1, 2, 3, 4, 8, 5, 10]
[1, 2, 3, 4, 8, 5, 10]
[1, 2, 3, 4, 5, 8, 10]
[1, 2, 3, 5, 4, 8, 10]
[1, 2, 3, 4, 5, 8, 10]
[1, 2, 3, 5, 4, 8, 10]
[1, 2, 3, 4, 5, 8, 10]
Cœur
  • 37,241
  • 25
  • 195
  • 267
  • You are being very complicated here, what are you trying to do? Your recursion is not ending so first is never equal to last. – Graeme Stuart Mar 10 '14 at 00:19

1 Answers1

1

The partition function seems fine to me. The main issues have to do with the selection function. They are:

  1. Mixing 0-indexing and 1-indexing
  2. Checking bounds of selection function
  3. Handling the k value for recursion

Point 1: mixing 0-indexing and 1-indexing

This example shows it:

print(selection([5,4,1,10,8,3,2], 0, 6, 1))

You said in your question that the expected output was 1. The list [5,4,1,10,8,3,2] sorted is [1,2,3,4,5,8,10] . In the call to the selection function, you supplied 0 and 6 as first and last respectively. These 2 variables use 0-indexing. For k however, you supplied 1 and expected the output of the selection function to be 1. This makes use of 1-indexing.

There's nothing wrong with that, but things can get confusing quickly. We should standardize things. I choose to use 0-indexing for k.

Point 2: Checking bounds of selection function

In particular, this statement:

if first == last:

should be changed to:

if first >= last:

because of the below statements:

elif k <= pivotIndex:
    return selection(a_list, first, pivotIndex-1, k)
else:
    return selection(a_list, pivotIndex+1, last, k - pivotIndex)

it is possible that first >= pivotIndex - 1 and pivotIndex + 1 >= last in either of those 2 recursive calls to selection. In those cases, we know that there is only 1 element remaining in the sublist, so we should just return it.

Point 3: Handling the k value for recursion

In this statement:

return selection(a_list, pivotIndex+1, last, k - pivotIndex)

It's not necessary to subtract pivotIndex from k. Even though the next selection call only takes into account of the sublist from pivotIndex+1 to last (inclusive), we are not creating a new array containing only the elements from a_list[pivotIndex+1] to a_list[last], and hence the element we are interested in will still be at position k.

With those changes

We can keep the partitionfunction as it is. Here's an updated selection function:

def selection(a_list, first, last, k):
    # Handle possibility that first >= last, so we only have
    # one element remaining in the sublist
    if first >= last:
        return a_list[k]
    pivot = a_list[last]
    pivotIndex = partition(a_list, first, last)
    if k < pivotIndex:
        return selection(a_list, first, pivotIndex-1, k)
    else:
        # k is left as it is
        return selection(a_list, pivotIndex+1, last, k)

You should change the calls to selection to use 0-indexing for k.

Hope that helps!

yanhan
  • 3,507
  • 3
  • 28
  • 38