What is wrong with this python function from "Programming Collective Intelligence"?

Question

This is the function in question. It calculates the Pearson correlation coefficient for p1 and p2, which is supposed to be a number between -1 and 1.

When I use this with real user data, it sometimes returns a number greater than 1, like in this example:

def sim_pearson(prefs,p1,p2):
    si={}
    for item in prefs[p1]: 
        if item in prefs[p2]: si[item]=1

    if len(si)==0: return 0

    n=len(si)

    sum1=sum([prefs[p1][it] for it in si])
    sum2=sum([prefs[p2][it] for it in si])

    sum1Sq=sum([pow(prefs[p1][it],2) for it in si])
    sum2Sq=sum([pow(prefs[p2][it],2) for it in si]) 

    pSum=sum([prefs[p1][it]*prefs[p2][it] for it in si])

    num=pSum-(sum1*sum2/n)
    den=sqrt((sum1Sq-pow(sum1,2)/n)*(sum2Sq-pow(sum2,2)/n))

    if den==0: return 0

    r=num/den

    return r

critics = {
    'user1':{
        'item1': 3,
        'item2': 5,
        'item3': 5,
        },

    'user2':{
        'item1': 4,
        'item2': 5,
        'item3': 5,
        }
}

print sim_pearson(critics, 'user1', 'user2', )

1.15470053838

Answer 1

It looks like you may be unexpectedly using integer division. I made the following change and your function returned 1.0:

num=pSum-(1.0*sum1*sum2/n)
den=sqrt((sum1Sq-1.0*pow(sum1,2)/n)*(sum2Sq-1.0*pow(sum2,2)/n))

See PEP 238 for more information on the division operator in Python. An alternate way of fixing your above code is:

from __future__ import division