Skip to content

Commit

Permalink
PERF: Updated lbfgsb to fix bug
Browse files Browse the repository at this point in the history
This patch was integrated from a change made in ITK:

commit 2194f1bf78eec5bea6f7c21758db08d0cc93e358
Author: Hans Johnson <hans-johnson@uiowa.edu>
Date:   Fri Dec 2 12:05:49 2011

    PERF: Updated lbfgsb to fix bug

    http://users.eecs.northwestern.edu/~nocedal/lbfgsb.html

    Condition for Use: This software is freely available, but we expect that
    all publications describing  work using this software , or all
    commercial products using it, quote at least one of the references given
    below. This software is released under the BSD License. This note has
    been added to the NOTICE file at the top of the ITK source tree.

    This remark describes an improvement and a correction to Algorithm 778.
    It is shown that the performance of the algorithm can be improved
    significantly by making a relatively simple modification to the subspace
    minimization phase. The correction concerns an error caused by the use
    of routine dpmeps to estimate machine precision.

    Change-Id: Iee35b8c6ee383b27a9870f73edbb604508c6ef2f

 .../VNL/src/vxl/core/vnl/algo/vnl_lbfgsb.cxx       |   12 +-
 .../ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.c | 3052 +++++---------------
 .../ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.h |   13 +-
 3 files changed, 731 insertions(+), 2346 deletions(-)

commit b3e618b485864db87530a50cb5d93e2d7141b7be
Author: Hans Johnson <hans-johnson@uiowa.edu>
Date:   Thu Dec 22 08:10:40 2011

    BUG: Remove uninitialized memory print.

    Valgrind exposed an uninitizialized memory print when printing
    diagnostics.

    ==13892== Conditional jump or move depends on uninitialised value(s)
    ==13892==    at 0x4C23DB8: strlen (mc_replace_strmem.c:242)
    ==13892==    by 0x5434A9E: vfprintf (in /lib64/libc-2.9.so)
    ==13892==    by 0x543AC69: printf (in /lib64/libc-2.9.so)
    ==13892==    by 0x4F4B2B0: v3p_netlib_prn3lb_ (lbfgsb.c:3439)
    ==13892==    by 0x4F4F025: v3p_netlib_mainlb_ (lbfgsb.c:1175)
    ==13892==    by 0x4F4FA21: v3p_netlib_setulb_ (lbfgsb.c:378)
    ==13892==    by 0x4009F7: main (lbfgsb-example1.c:176)

    Change-Id: Iae022fa374326d045bcd9f95c5de99260eb1b9d4

 Modules/ThirdParty/VNL/src/vxl/v3p/netlib/tests/lbfgsb-example1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit 8b2a376add6059d1fe2f0f24b849e9f7f715e492
Author: Bill Lorensen <bill.lorensen@gmail.com>
Date:   Fri Dec 30 10:33:15 2011

    COMP: Unterminated string causes valgrind defect.

    NOTE: these changes were committed in Dec 2010 to the previous version
    of lbfgsb.

    The netlib s_copy procedure copies n characters from one string into
    another string. Just like strncpy, the null termination character is
    not copied unless it is within the first n characters of the source
    string. In lbfgsb.c, s_copy is used to updated a char[60] variable
    called task. This variable, when used in a printf statement, will
    cause a defect:

    "Conditional jump or move depends on uninitialised value(s)"

    This patch adds 1 to each s_copy(task,...) call so that the null
    termination from the literal string will be included in the copy.

    Change-Id: Ib1753e61b5ca9d2a066d09cbb1d124321c017707

 .../ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.c | 62 +++++++++++-----------
 1 file changed, 31 insertions(+), 31 deletions(-)

commit 24fc4fe4270b39b8ec906213f06293102ebe1401
Author: Ali Ghayoor <ali-ghayoor@uiowa.edu>
Date:   Tue May 13 13:11:38 2014

    BUG: lbfgsb optimizer could not be used in unbounded mode

    LBFGSB optimizer can put lower or upper or both constrains on selective
    variables. However, it also can be used in unbounded mode.

    There were some issues that prevented LBFGSB to be used in unbounded
    mode. This patch resolves these issues and expands the
    itkLBFGSBOptimizerv4Test to check the unbounded LBFGSB too.

    Change-Id: Id36b39a1fe500d449bbaee25b886ecae608355f5

 Modules/ThirdParty/VNL/src/vxl/core/vnl/algo/vnl_lbfgsb.cxx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit feedd5b37187e29588bbf15ebbea582f626987c4
Author: Bradley Lowekamp <blowekamp@mail.nih.gov>
Date:   Tue Apr 21 13:26:01 2015

    BUG: Correct convergence string check

    Use conventions for strings from v3p_netlib_setulb_

    Change-Id: Ida814973ec6e6a5849d1b2484b9581fd1239856e

 Modules/ThirdParty/VNL/src/vxl/core/vnl/algo/vnl_lbfgsb.cxx | 2 +-
 Modules/ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

commit e29bf9abb3f76871f8d726849a57fab70e6914e6
Author: Tobias Wood <tobias@spinicist.org.uk>
Date:   Wed Jul 22 14:48:44 2015

    BUG: LBFGSB was printing messages even with debug switched off.

    Change-Id: I1fb1b980c2e2fd293d29f291b48ed026324ea661

    This bug was also reported in scipy - see here scipy/scipy#2261

 Modules/ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

commit 192495ac402821fcdcb10afaf14003494c21f227
Author: Tobias Wood <tobias@spinicist.org.uk>
Date:   Fri Jul 31 16:20:09 2015

    BUG: Removed an unguarded print statement.

    This print statement is in a function that does not get passed the
    iprint debugging variable. To avoid significant changes to the code
    (e.g. adding the iprint variable as an argument to the function) I
    simply commented it out.

    Change-Id: I62a7c2401e503578b996a2fc9729cae257450d78

 Modules/ThirdParty/VNL/src/vxl/v3p/netlib/opt/lbfgsb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  • Loading branch information
hjmjohnson committed Nov 15, 2015
1 parent 3bb65c1 commit 3cdc40c
Show file tree
Hide file tree
Showing 4 changed files with 1,949 additions and 3,562 deletions.
14 changes: 12 additions & 2 deletions core/vnl/algo/vnl_lbfgsb.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,17 @@ bool vnl_lbfgsb::minimize(vnl_vector<double>& x)
vnl_vector<double> gradient(n);

// Working space.
vnl_vector<double> wa((2*m+4)*n + 12*m*m + 12*m);
// The total work space **wa** required by the new version is
//
// 2*m*n + 11*m*m + 5*n + 8*m
//
vnl_vector<double> wa(2*m*n + 11*m*m + 5*n + 8*m);
//
// the previous version required:
//
// 2*m*n + 12*m*m + 4*n + 12*m
//
//
vnl_vector<long> iwa(3*n);
char csave[60];
long lsave[4];
Expand Down Expand Up @@ -150,7 +160,7 @@ bool vnl_lbfgsb::minimize(vnl_vector<double>& x)
// function tolerance reached
this->failure_code_ = CONVERGED_FTOL;
}
else if (vcl_strncmp("CONVERGENCE: NORM OF PROJECTED GRADIENT <= PGTOL",
else if (vcl_strncmp("CONVERGENCE: NORM_OF_PROJECTED_GRADIENT_<=_PGTOL",
task, 48) == 0)
{
// gradient tolerance reached
Expand Down

0 comments on commit 3cdc40c

Please sign in to comment.