Search code examples
pythoncalgorithmdivide-and-conquer

Python translation of C code not working


I want to find the maximum element in an array in O(log N) time using divide and conquer. I found a working code at planet-source-code. I translated it into Python as follows:

def largest(arr,i,j):
    global max1
    global max2
    if i == j:
        max1 = arr[i]
    else:
        if i == j-1:
           max1 = arr[i] if arr[i] > arr[j] else arr[j]
        else:
              mid = (i+j)/2
              largest(arr,i,mid)
              max2 = max1
              largest(arr,mid+1,j)
              if max2 > max1:
             max1 = max2

When I use an array [98,5,4,3,2,76,78,92], and call the code as

max1 = arr[0]
largest(arr,1,8)

I am getting a out-of-bound list index error. However the C code returns the correct result 98. Can anyone spot what error am I doing?


Solution

  • For a general unsorted array, you can never find the max in anything less than O(n) time. The very trivial proof: if you do it in less than O(n) time, then for a sufficiently large array you don't have enough time to inspect every element. An adversary can therefore put the maximum value in an element you don't check, making your algorithm incorrect.

    What the original code is good for is finding both the maximum and minimum simultaneously using fewer than 2n comparisons (as the naive implementation would do) -- it uses approximately 1.5n comparisons since it performs only one comparison when there are two elements. You derive no benefit from using it to only find the maximum: you'd be better off using max(arr) in Python instead (which would also be faster since it has no function call overhead).

    The original code stores the values in a[1] through a[n], which requires an array of size n+1. Therefore you should put a dummy element in the first position.

    But, more problematically, your translation is incorrect. The original uses globals to effect a multiple value return (which is an incredibly hacky way to do it), and local variables to save the old globals. Since you make both max1 and max2 global, the function will not produce the right answer anyway.

    The correct translation to Python would use a direct multiple value return with a tuple:

    def minmax(arr, i, j):
        if i==j:
            return arr[i], arr[i]
        elif i==j-1:
            if arr[i] < arr[j]:
                return arr[i], arr[j]
            else:
                return arr[j], arr[i]
        else:
            mid = (i+j)//2
            min1, max1 = minmax(arr, i, mid)
            min2, max2 = minmax(arr, mid+1, j)
            if min2 < min1: min1 = min2
            if max2 > max1: max1 = max2
            return min1, max1