Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(10518)

Issue 52096: Refactor line search functions out of optimize.py

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 11 months ago by pav
Modified:
9 years, 4 months ago
Reviewers:
stefanv
Visibility:
Public.

Description

Refactor line search functions out of optimize.py Also, split each line search function f(x + s*p) -> suitable minimum to a scalar search function phi(s) -> suitable minimum and a wrapper phi_p(s) = f(x + s*p) that makes it a line search. This refactoring is necessary when we want to use the *same* line search functions we use in optimization in solving nonlinear equations. This is because in large-scale non-linear equations we want to minimize f(x) = |F(x)|_2^2, but it is impossible to compute the gradient f'(x) = J(x)^T F(x), even though it is easy to compute the derivative phi'(s) = d phi(s)/ds directly [eg. by numerical differentiation]. Tests for line searches are also added. These basically check that they do what they promise, for a couple of problems. Not all corner cases are still tested, though. Also added a couple of tests that check eg. that fmin_bfgs and other routines that use these line search routines - produce the same sequence of x_k as before - require the same number of function calls as before This tries to ensure that these changes to subroutines doesn't change the behavior of the top-level routines. However, I have the feeling that the test coverage is not yet really sufficient; the test_optimize.py tests don't offer 100% coverage of the optimize.py code branches.

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+864 lines, -358 lines) Patch
M scipy/optimize/linesearch.py View 1 chunk +550 lines, -34 lines 4 comments Download
M scipy/optimize/optimize.py View 4 chunks +9 lines, -324 lines 0 comments Download
A scipy/optimize/tests/test_linesearch.py View 1 chunk +234 lines, -0 lines 0 comments Download
M scipy/optimize/tests/test_optimize.py View 8 chunks +71 lines, -0 lines 0 comments Download

Messages

Total messages: 1
stefanv
14 years, 11 months ago (2009-05-03 23:44:15 UTC) #1
Good to see the improved test coverage.  I scanned through the refactored code
-- looks good!

Some of the older code (I assume not written by you) still uses the old
docstring format; may be worth fixing.  I marked these, but of course it is not
your responsibility to clean up!

The 'zoom' function also came from the older code but lacks a docstring; if you
know what it does, could you add a one line description?

http://codereview.appspot.com/52096/diff/1/2
File scipy/optimize/linesearch.py (right):

http://codereview.appspot.com/52096/diff/1/2#newcode411
Line 411: phi, derphi, phi0, derphi0, c1, c2):
We should document zoom, if it's for public consumption.

http://codereview.appspot.com/52096/diff/1/2#newcode443
Line 443: #                print "Using bisection."
These debugging prints can be removed.

http://codereview.appspot.com/52096/diff/1/2#newcode492
Line 492: :Returns: (alpha, f_count, f_val_at_alpha)
Old docstring format.

http://codereview.appspot.com/52096/diff/1/2#newcode525
Line 525: :Returns: alpha, phi1
Old docstring format.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b