python merge sort issue

Question

Not sure where I am going wrong with my implementation of merge sort in python.

import sys

sequence = [6, 5, 4, 3, 2, 1]

def merge_sort(A, first, last):
    if first < last:
        middle = (first + last) / 2
        merge_sort(A, first, middle)
        merge_sort(A, middle+1, last)
        merge(A, first, middle, last)

def merge(A, first, middle, last):
    L = A[first:middle]
    R = A[middle:last]

    L.append(sys.maxint)
    R.append(sys.maxint)

    i = 0
    j = 0
    for k in xrange(first, last):
        if L[i] <= R[j]:
            A[k] = L[i]
            i = i + 1
        else:
            A[k] = R[j]
            j = j + 1

merge_sort(sequence, 0, len(sequence))
print sequence

I would really appreciate it if someone could point out what is breaking my current implementation of merge sort.

I've literally attempted to implement the pseudocode outlined in the CLRS (Introduction to Algorithms) textbook. Not sure where I'm going wrong :( — nuce, Jan 26 '16 at 12:37
When running your code, I notice it first tries to sort [6]. Then it sorts [4]. Fine thus far. But then, it tries to sort [6,5,4]. It seems it should sort [6,5] or [5,4] first. — Christofer Ohlsson, Jan 26 '16 at 13:06

score 5 · Answer 1 · answered Jan 26 '16 at 13:05

5

There are 2 errors in the code:

if first < last: should be if first < last and last-first >= 2:
merge_sort(A, middle+1, last) should be merge_sort(A, middle, last)

answered Jan 26 '16 at 13:05

Anmol Singh Jaggi

8,376
4
36
77

1

Isn't the first point merely an optimization? As long as the `middle` calculation rounds down, the code isn't incorrect. – Mark Ransom Jan 26 '16 at 13:11
That seems to have fixed it, but I am still struggling to see how this additional check and modification of the middle index has fixed the original implementation :( Apologies if this seems dumb, but I would have expected the textbook to include additional details if this is the case. – nuce Jan 26 '16 at 13:13
3

Earlier, the middle element wasn't being included in any of the two halves. So, changing `middle+1` to `middle` includes it in the right half. – Anmol Singh Jaggi Jan 26 '16 at 13:16
2

`if first < last - 1:` is a bit shorter and IMHO clearer. – tobias_k Jan 26 '16 at 14:12
@nuce Feel free to ask any more questions if you are still not clear. If you are satisfied, then you can close the question by accepting the answer. – Anmol Singh Jaggi Jan 26 '16 at 15:17
@AnmolSinghJaggi Thanks, middle element caught me off guard. I need to be careful with indexes. – nuce Jan 27 '16 at 10:53

score 5 · Accepted Answer · answered Jan 26 '16 at 13:25

The problem is here:

    merge_sort(A, first, middle)
    merge_sort(A, middle+1, last) # BEEP

You only sort the second part from middle + 1, when you should start at middle. In fact, you never reorder the element at middle.

Of course you cannot write either

    merge_sort(A, first, middle)
    merge_sort(A, middle, last) # BEEP, BEEP

because when last = first + 1, you get middle == first and dive in an endless recursion (stopped by a RuntimeError: maximum recursion depth exceeded)

So the way to go is:

    merge_sort(A, first, middle)
    if middle > first: merge_sort(A, middle, last)

After that little change, your implementation gives correct results.

Thanks, this is a clear answer and I'm glad I can now recognize that the middle index was the main cause. Both your answer and Anmol helped. — nuce, Jan 27 '16 at 10:55

python merge sort issue

2 Answers2