ENH: use memoization in MINPACK routines #236

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants
@dlax
Member

dlax commented Jun 4, 2012

This implements memoization of the objective function and Jacobian for MINPACK routines.
See #130 for a rationale and use-case.

@pv

pv reviewed Jun 4, 2012
View changes

scipy/optimize/optimize.py
+ self.f = self.fun(x, *args)
+ self.calls += 1
+ return self.f
+ elif self.first_only:

This comment has been minimized.

@pv

pv Jun 4, 2012

Member

I think this doesn't work --- if first_only is True, the cached function value is never used.

@pv

pv Jun 4, 2012

Member

I think this doesn't work --- if first_only is True, the cached function value is never used.

This comment has been minimized.

@dlax

dlax Jun 4, 2012

Member

Pauli Virtanen a écrit :

I think this doesn't work --- if first_only is True, the cached function value is never used.

You're right. In fact, in the original version (tested in #130) the
second and third elif clauses were swapped and it works that way. I'll
look into this again...

@dlax

dlax Jun 4, 2012

Member

Pauli Virtanen a écrit :

I think this doesn't work --- if first_only is True, the cached function value is never used.

You're right. In fact, in the original version (tested in #130) the
second and third elif clauses were swapped and it works that way. I'll
look into this again...

This comment has been minimized.

@dlax

dlax Jun 4, 2012

Member
def __call__(self, x, *args):
    if not hasattr(self, 'x'):
        self.x = numpy.asarray(x).copy()
        self.f = self.fun(x, *args)
        self.calls = 1
        return self.f
    elif self.first_only and self.calls > 1:
        return self.fun(x, *args)
    elif numpy.any(x != self.x):
        self.x = numpy.asarray(x).copy()
        self.f = self.fun(x, *args)
        self.calls += 1
        return self.f
    else:
        return self.f

Does this look better?

@dlax

dlax Jun 4, 2012

Member
def __call__(self, x, *args):
    if not hasattr(self, 'x'):
        self.x = numpy.asarray(x).copy()
        self.f = self.fun(x, *args)
        self.calls = 1
        return self.f
    elif self.first_only and self.calls > 1:
        return self.fun(x, *args)
    elif numpy.any(x != self.x):
        self.x = numpy.asarray(x).copy()
        self.f = self.fun(x, *args)
        self.calls += 1
        return self.f
    else:
        return self.f

Does this look better?

This comment has been minimized.

@dlax

dlax Jun 4, 2012

Member

fixed in a16c58a

@dlax

dlax Jun 4, 2012

Member

fixed in a16c58a

@rgommers

This comment has been minimized.

Show comment
Hide comment
@rgommers

rgommers Jun 4, 2012

Member

OK, I'll wait; let me know when it's ready.

Member

rgommers commented Jun 4, 2012

OK, I'll wait; let me know when it's ready.

dlax added some commits Jun 4, 2012

FIX: copy __name__ attribute upon memoization in optimize
This is needed since, for some kinds of failures, MINPACK routines output
an error message which refers to the __name__ attribute of the objective
function and jacobian.
@dlax

This comment has been minimized.

Show comment
Hide comment
@dlax

dlax Jun 5, 2012

Member

I've rebased the branch in order to ease merge.
It is ready now, I think.

Member

dlax commented Jun 5, 2012

I've rebased the branch in order to ease merge.
It is ready now, I think.

@yosefm

This comment has been minimized.

Show comment
Hide comment
@yosefm

yosefm Jun 7, 2012

Tested, it is slower than vanilla SciPy for me.
I have to apologize - when I tested in #130 I made a PYTHONPATH mistake, so I actually didn't test your version there. But now it's ok.

yosefm commented Jun 7, 2012

Tested, it is slower than vanilla SciPy for me.
I have to apologize - when I tested in #130 I made a PYTHONPATH mistake, so I actually didn't test your version there. But now it's ok.

@dlax

This comment has been minimized.

Show comment
Hide comment
@dlax

dlax Jun 7, 2012

Member

Yosef Meller a écrit :

Tested, it is slower than vanilla SciPy for me.
I have to apologize - when I tested in #130 I made a PYTHONPATH mistake, so I actually didn't test your version there. But now it's ok.

So you mean it does not solve your problem?
If so, providing a minimal example would help at this point.

Member

dlax commented Jun 7, 2012

Yosef Meller a écrit :

Tested, it is slower than vanilla SciPy for me.
I have to apologize - when I tested in #130 I made a PYTHONPATH mistake, so I actually didn't test your version there. But now it's ok.

So you mean it does not solve your problem?
If so, providing a minimal example would help at this point.

@pv

This comment has been minimized.

Show comment
Hide comment
@pv

pv Jun 7, 2012

Member

Yosef's problem is probably mainly Python overhead. Adding caching however will be useful in the opposite case when the objective function is slow. One can however make the memoization still faster by just adding the assumption that the first two calls are at same point, rather than explicitly checking for this condition which is always true...

Member

pv commented Jun 7, 2012

Yosef's problem is probably mainly Python overhead. Adding caching however will be useful in the opposite case when the objective function is slow. One can however make the memoization still faster by just adding the assumption that the first two calls are at same point, rather than explicitly checking for this condition which is always true...

@dlax

This comment has been minimized.

Show comment
Hide comment
@dlax

dlax Jun 7, 2012

Member

One can however make the memoization still faster by just adding the assumption that the first two calls are at same point, rather than explicitly checking for this condition which is always true...

I don't understand this. What do you suggest?

If this were to be dropped, note (to self as well) that cd8ba57 contains a fix that has to be applied anyways.

Member

dlax commented Jun 7, 2012

One can however make the memoization still faster by just adding the assumption that the first two calls are at same point, rather than explicitly checking for this condition which is always true...

I don't understand this. What do you suggest?

If this were to be dropped, note (to self as well) that cd8ba57 contains a fix that has to be applied anyways.

@pv

This comment has been minimized.

Show comment
Hide comment
@pv

pv Jun 7, 2012

Member

@dlaxalde: assume that np.all(x == self.x) is true for the first call. The optimization algorithms probably always first evaluate the function value at the input point, so there is no need to actually check what the input argument actually is.

Member

pv commented Jun 7, 2012

@dlaxalde: assume that np.all(x == self.x) is true for the first call. The optimization algorithms probably always first evaluate the function value at the input point, so there is no need to actually check what the input argument actually is.

@dlax

This comment has been minimized.

Show comment
Hide comment
@dlax

dlax Jun 7, 2012

Member

assume that np.all(x == self.x) is true for the first call. The optimization algorithms probably always first evaluate the function value at the input point, so there is no need to actually check what the input argument actually is.

This is skipped in the first call since self.calls == 0 (self.x does not exist yet btw).

Member

dlax commented Jun 7, 2012

assume that np.all(x == self.x) is true for the first call. The optimization algorithms probably always first evaluate the function value at the input point, so there is no need to actually check what the input argument actually is.

This is skipped in the first call since self.calls == 0 (self.x does not exist yet btw).

@pv

This comment has been minimized.

Show comment
Hide comment
@pv

pv Jun 7, 2012

Member

07.06.2012 20:46, Denis Laxalde kirjoitti:

assume that np.all(x == self.x) is true for the first call.
The optimization algorithms probably always first evaluate the
function value at the input point, so there is no need to actually
check what the input argument actually is.

This is skipped in the first call since self.calls == 0 (self.x does not exist yet btw).

But it is not skipped in the first call from the optimization routine
itself, i.e., the second call.

Checking np.all(x == self.x) for this call is probably unnecessary as
the optimization algorithm will always evaluate f(x0) first. Btw, this
called-twice-at-same-point pattern is more or less what the skip_check
option tries to work around --- however, working around is not necessary
as this can be fixed in the wrapper.

Member

pv commented Jun 7, 2012

07.06.2012 20:46, Denis Laxalde kirjoitti:

assume that np.all(x == self.x) is true for the first call.
The optimization algorithms probably always first evaluate the
function value at the input point, so there is no need to actually
check what the input argument actually is.

This is skipped in the first call since self.calls == 0 (self.x does not exist yet btw).

But it is not skipped in the first call from the optimization routine
itself, i.e., the second call.

Checking np.all(x == self.x) for this call is probably unnecessary as
the optimization algorithm will always evaluate f(x0) first. Btw, this
called-twice-at-same-point pattern is more or less what the skip_check
option tries to work around --- however, working around is not necessary
as this can be fixed in the wrapper.

@dlax

This comment has been minimized.

Show comment
Hide comment
@dlax

dlax Jun 7, 2012

Member

Please see 3d24d3b.
Not sure it will solve your problem @yosefm but at least it skips one more call...

Member

dlax commented Jun 7, 2012

Please see 3d24d3b.
Not sure it will solve your problem @yosefm but at least it skips one more call...

@pv pv added the PR label Feb 19, 2014

@pv pv removed the PR label Aug 13, 2014

@dlax dlax closed this Jan 11, 2015

@dlax dlax deleted the dlax:enh/optimize/memoize-minpack branch Jan 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment