0

I've got to build a complete MIN-HEAP implementation in Python, without using built-in heap functions.

So I've got definitions of parent, left child and right child, which take in account that python number list elements from 0:

from random import randint
import math 

def parent(i):
    x = int(l.index(l[i])) #########3
    y = int(math.floor(x/2))
    return y-1  

def lchild(i):
    x = int(l.index(l[i]))
    y = int(math.floor(x*2))
    return y-1

def rchild(i):
    x = int(l.index(l[i]))
    y = int(math.floor(x*2 + 1))
    return y-1

then I have a part of code that generates (pseudo)random list for me into the list l:

l = []
dl = int(input)

for i in range (0, dl):
    x = int(randint(1,100)) 
    l.append(x)             

and until this point everything works good. then I have a function bkop for making the table l into a min-heap.

def bkop(l):
        j = 0
        for i in range(0, len(l)):
                if int(l[i]) < int(parent(l[i])): #########2
                        l[i], l[parent(i)] = l[parent(i)], l[i]
                        j = j+1
                if j != 0:
                        bkop(l)

then I want to run a program and see the results:

bkop(l) #########1
print l

The program crashes with an error list index out of range pointing to the 3 lines, that i've marked with #########. I've started writing this about a month ago and i'm pretty sure, that parent, lchild and rchild worked at that time. Do you know, what's wrong?

EDIT1: Ok, so I've fixed the parent/lchild/rchild definitions. I've checked, they return correct values. Here is the code:

def parent(i):
    x = i + 1
    y = x//2
    return y-1

def lchild(i):
    x = i + 1
    y = x*2
    return y-1

def rchild(i):
    x = i + 1
    y = x*2 + 1
    return y-1

The function generating random list is pretty much intact. Then I have this bkop function (that makes a min-heap out of the l list). I use print before and after on purpose, to se if it works... and it doesn't. It returns the same list both times, no compile errors or anything. Any idea how to fix it?

print(l)
def bkop(l): 
        j = 0
        for i in range(0, len(l)):
                if l[i] < parent(i):
                        l[i], l[parent(i)] = l[parent(i)], l[i]
                        j = j+1
                if j != 0:
                        bkop(l)
bkop(l)
print l

EDIT2: Ok, so I've fixed bkop as you've suggested:

print bkop(l)
def bkop(l):
        j = 0
        for i in range(1, len(l)):
                if l[i] < l[parent(i)]:
                        l[i], l[parent(i)] = l[parent(i)], l[i]
                        j = j+1
                if j != 0:
                        bkop(l)
bkop(l)
print bkop(l)

But when I run it, I get this first a randomly-generated table (as I should), and instead of a min-heap table, I get None value, as below:

[34, 9, 94, 69, 77, 33, 56]
None

Any ideas? Maybe I should do it another way around, not comparing l[i] to parent, but l[i] to it's left and right children?

kjubus
  • 443
  • 3
  • 8
  • 21

2 Answers2

3

So I've got the whole thing working with the help from Wombatz (Thank you very much!). Here is the working code for the future users:

from random import randint

def parent(i):
    x = i + 1
    y = x//2
    return y-1

def lchild(i):
    x = i + 1
    y = x*2
    return y-1

def rchild(i):
    x = i + 1
    y = x*2 + 1
    return y-1



l = []
dl = int(input())

for i in range (0, dl):
    x = int(randint(1,100)) 
    l.append(x) 

print l
def bkop(l):
        j = 0
        for i in range(1, len(l)):
                if l[i] < l[parent(i)]:
                        l[i], l[parent(i)] = l[parent(i)], l[i]
                        j = j+1
                if j != 0:
                        bkop(l)
bkop(l)
print l

When run, I've put 13 in input for the list length and i've got the result:

13
[30, 62, 9, 100, 75, 73, 57, 82, 2, 76, 2, 50, 41] #before heapify
[2, 2, 30, 62, 9, 41, 57, 100, 82, 76, 75, 73, 50] #after heapify
kjubus
  • 443
  • 3
  • 8
  • 21
2

At first: there is a problem with your parent, lchild and rchild functions:

  • l[index] gets the value for a given index.

  • l.index(...) gets the index for a given value.

Your are trying to get the index for a value at a specific index. That's like x + 1 - 1. If your items are unique then that computed index will always be the same as the index you started with. When there are duplicates your functions will calculate the parent, left and right child of the first occurence of the value at that index. That is probably not what you want. So set x to i or remove the x completely.

The actual problem is the following:

Your parent, lchild and rchild functions are desined to work on an index but you are passing the value of l at index i to the function: parent(l[i])

Since the values in the list may be much higher than the possible index range you are getting list index out of range. You probably want to pass the index directly.

Furthermore:

parent, lchild and rchild return incorrect values. Test these function without all the other stuff!

I assume the results should look like this:

  • parent(1) == parent(2) == 0
  • lchild(0) == 1
  • rchild(0) == 2

Your definition returns different values.

Some minor things:

  • All these random int casts are useless. Casting an int to int does nothing. The only line which really need the cast is: ``dl = int(input)`
  • int(math.floor(x/2)). Use integer division x // 2 instead
  • int(math.floor(x*2)). Integer times 2 is an integer. No need to floor it.

Edit Two things are wrong with your bkop function.

  • l[i] < parent(i) You compare the value to an index. I think you want to campare the value at i to its parent value, not the parent index.
  • for i == 0 you compare the value at index 0 to the value at parent(0) == -1. This wraps around the list and is probably not what you want. As index 0 has no parent you shouldn't try to compare it to its parent.

Edit2

bkop does not return the list but modifies it in-place. Just print(l) at the end. You will see that the original list was modified.

Wombatz
  • 4,958
  • 1
  • 26
  • 35
  • Thank you for your hints. I've cleaned up parent/lchild/rchild definitions and checked - they work good (return the index in table) But I still have problem with my **bkop** function. Could you please see EDIT1 to my original post? – kjubus Jan 23 '16 at 15:47
  • Thank you, I've overlooked those mistakes. But I still didn't get it working. Could you please look at EDIT 2? – kjubus Jan 23 '16 at 16:35
  • @Edit 2 - that didn't work. I still het **None** returned – kjubus Jan 23 '16 at 16:51
  • It is not necessary to return anything. The result is in `l`. Just call `bkop(l)` without return or assignment. `l` is a heap then. – Wombatz Jan 23 '16 at 16:59
  • I do call **bkop(l)** without return or assignment. I call print(l) AFTER the loop is done, and it prints None: `[44, 59, 42, 71, 61, 96]` **#before the bkop** `None` **#after bkop** – kjubus Jan 23 '16 at 19:16
  • Make sure that there is not assignment to `l` outside `bkop` – Wombatz Jan 23 '16 at 19:38
  • I did. You can see the enirety of the code above. First I create an empty l = [] list, then I randomly fill it. at the end i use **bkop(l)** and when I try to **print(l)**, instead of a list like [3, 5, 7, 6, 8, 9, 13] i just get "None" – kjubus Jan 23 '16 at 20:39
  • Don't `print bkop(l)`. Just `print l` – Wombatz Jan 23 '16 at 20:51
  • I know, that's exactly what I do (there has been a slight change in the code above) – kjubus Jan 23 '16 at 20:59
  • Ok, I've got it working, I did a small mistake before. I will put a working code in EDIT3 – kjubus Jan 23 '16 at 21:06