views:

624

answers:

6
+5  Q: 

Closures in Python

Hi, I've been trying to learn Python, and while I'm enthusiastic about using closures in Python, I've been having trouble getting some code to work properly:

def memoize(fn):
    def get(key):
        return (False,)

    def vset(key, value):
        global get
        oldget = get
        def newget(ky):
            if key==ky: return (True, value)
            return oldget(ky)
        get = newget

    def mfun(*args):
        cache = get(args)
        if (cache[0]): return cache[1]

        val = apply(fn, args)
        vset(args, val)
        return val

    return mfun

def fib(x):
    if x<2: return x
    return fib(x-1)+fib(x-2)

def fibm(x):
    if x<2: return x
    return fibm(x-1)+fibm(x-2)

fibm = memoize(fibm)

Basically, what this is supposed to do is use closures to maintain the memoized state of the function. I realize there are probably many faster, easier to read, and in general more 'Pythonic' ways to implement this; however, my goal is to understand exactly how closures work in Python, and how they differ from Lisp, so I'm not interested in alternative solutions, just why my code doesn't work and what I can do (if anything) to fix it.

The problem I'm running into is when I try to use fibm - Python insists that get isn't defined:

Python 2.6.1 (r261:67515, Feb  1 2009, 11:39:55) 
[GCC 4.0.1 (Apple Inc. build 5488)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import memoize
>>> memoize.fibm(35)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "memoize.py", line 14, in mfun
    cache = get(args)
NameError: global name 'get' is not defined
>>>

Seeing as I'm new to Python, I don't know if I've done something wrong, or if this is just a limitation of the language. I'm hoping it's the former. :-)

A: 

You want to put global get at the beginning of every function (except get itself).

the def get is an assignment to the name get, so you want get to be declared global before that.

Putting global get in mfun and vset makes them work. I can't point to the scoping rules that makes this necessary, but it works ;-)

Your conses are quite lispy too... :)

Jonas Kölker
Hmm.. on my machine, putting global get in get and vset didn't work. Adding it to the beginning of memoize itself worked, but that causes every invocation of memoize to use the same get function (not desirable).
Kyle Cronin
ah yeah. Note to self: drink coffee, wake up, *then* answer ;-)
Jonas Kölker
+6  A: 

The problem is in your scoping, not in your closures. If you're up for some heavy reading, then you can try http://www.python.org/dev/peps/pep-3104/.

If that's not the case, here's the simple explanation:

The problem is in the statement global get . global refers to the outermost scope, and since there isn't any global function get, it throws.

What you need, is an access specifier for variables in the enclosing scope, and not the global scope.

In python 3.0, as I've tested, the nonlocal keyword is exactly what you need, in the place of global.

nonlocal get
...

In python 2.x, I just removed the global get and the oldget references and it works properly.

sykora
So it's a flaw in the language. I was hoping you weren't going to say that. :-( A possible workaround is to store the value in a mutable structure (like a list) and modify that, but that's pretty hackish. I wanted a 2.X release for compatibility, but I might have to make the jump to 3000.
Kyle Cronin
There is a workaround, a fairly simple one at that, unless it messes with your concept of purity -- Simply make get() a global function.That said, I must say that your method of memoization seems too complicated for me :)
sykora
How does it work properly? Only removing "global get" and "oldget = get" lines and it never uses a saved value.
Roger Pate
And changing "oldget" to "get" in newget, that is.
Roger Pate
I assume that by "works properly", he means that it returned the correct answer (albeit without memoization).
Kyle Cronin
It took me a while to figure out that it wasn't getting memoized, but yes, that's what I meant. I'm trying to get it to work with memoization, but I don't understand the code sufficiently to find out what it's doing.
sykora
The basic idea is that, initially, get always returns (false, none) for every key because it doesn't have any record of it available. Then every time vset is called, a new get is created that tests the key with the one provided to vset. If it doesn't match, it calls the old get.
Kyle Cronin
More or less it becomes a chain of closures, with each new get having a reference to the old gets.
Kyle Cronin
A: 

Probably because you want the global get while it isn't a global? By the way, apply is deprecated, use fn(*args) instead.

def memoize(fn):
    def get(key):
        return (False,)

    def vset(key, value):
        def newget(ky):
            if key==ky: return (True, value)
            return get(ky)
        get = newget

    def mfun(*args):
        cache = get(args)
        if (cache[0]): return cache[1]

        val = fn(*args)
        vset(args, val)
        return val

    return mfun

def fib(x):
    if x<2: return x
    return fib(x-1)+fib(x-2)

def fibm(x):
    if x<2: return x
    return fibm(x-1)+fibm(x-2)

fibm = memoize(fibm)
Florian Mayer
Thanks for the tip about apply. However, running the modified code above works in the sense that it gives the correct answer, but it doesn't use the memoization at all. (you can verify this by running something like fibm(35), which, if memoized, should be near instantaneous.
Kyle Cronin
) <-- close paren ;-)
Kyle Cronin
x = y assigns to (or creates) the local variable x, unless there is a global-statement somewhere in that function.
MizardX
+7  A: 
def memoize(fn):
  get = [lambda key: (False, None)]

  def vset(args):
    value = fn(*args)
    oldget = get[0]
    def newget(key):
      if args == key:
        return (True, value)
      return oldget(key)
    get[0] = newget
    return value

  def mfun(*args):
    found, value = get[0](args)
    if found:
      return value
    return vset(args)

  return mfun

CALLS = 0

def fib(x):
  global CALLS
  CALLS += 1
  if x<2: return x
  return fib(x-1)+fib(x-2)

@memoize
def fibm(x):
  global CALLS
  CALLS += 1
  if x<2: return x
  return fibm(x-1)+fibm(x-2)

CALLS = 0
print "fib(35) is", fib(35), "and took", CALLS, "calls"
CALLS = 0
print "fibm(35) is", fibm(35), "and took", CALLS, "calls"

Output is:

fib(35) is 9227465 and took 29860703 calls
fibm(35) is 9227465 and took 36 calls

Similar to other answers, however this one works. :)

The important change from the code in the question is assigning to a non-global non-local (get); however, I also made some improvements while trying to maintain your *cough*broken*cough* closure use. Usually the cache is a dict instead of a linked list of closures.

Roger Pate
It's not ideal, but the workaround works well enough I suppose. Thanks for polishing the code a bit too.
Kyle Cronin
IMHO, using this code for anything more than learning about how Python handles closures is doomed. The other changes are more to push you in the right direction than anything else.
Roger Pate
That was the main goal. If I were coding it in Python, I would use dictionaries instead, but I do appreciate little touches like setting multiple values in one statement and using fn(*args) instead of apply.
Kyle Cronin
+1  A: 

Get is not global, but local to the surrounding function, that's why the global declaration fails.

If you remove the global, it still fails, because you can't assign to the captured variable name. To work around that, you can use an object as the variable captured by your closures and than just change properties of that object:

class Memo(object):
    pass

def memoize(fn):
    def defaultget(key):
        return (False,)

    memo = Memo()
    memo.get = defaultget

    def vset(key, value):
        oldget = memo.get
        def newget(ky):
            if key==ky: return (True, value)
            return oldget(ky)
        memo.get = newget

    def mfun(*args):
        cache = memo.get(args)
        if cache[0]: return cache[1]

        val = apply(fn, args)
        vset(args, val)
        return val

    return mfun

This way you don't need to assign to the captured variable names but still get what you wanted.

sth
A: 

I think the best way would be:

class Memoized(object):
    def __init__(self,func):
        self.cache = {}
        self.func = func
    def __call__(self,*args):
        if args in self.cache: return cache[args]
        else:
            self.cache[args] = self.func(*args)
            return self.cache[args]
ooboo