Skip to content
This repository was archived by the owner on Nov 23, 2018. It is now read-only.
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions bisection.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,38 @@

package optimize

import "math"
import (
"math"

"github.com/gonum/floats"
)

// Bisection is a Linesearcher that uses a bisection to find a point that
// satisfies the strong Wolfe conditions with the given gradient constant and
// function constant of zero. If GradConst is zero, it will be set to a reasonable
// value. Bisection will panic if GradConst is not between zero and one.
// EqTol is used to smooth out floating point noise when testing the Wolfe
// conditions. The Wolfe condition will be considered met if the function value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would replace Wolfe with sufficient decrease or Armijo in these two lines.

// is less than or within a tolerance of the starting value of the line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true, we compare to the value on the left side of the interval (Is it even correct? The original code compared to the lowest value found).

There is not a suitable line to put the comment on, but b.initF field and minF variable are not used in this version, is it on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, should be minF.

// search. If EqTol is zero, it will be defaulted to 1e-10, and if it is negative
// a strict less than or equal to comparison will be used. A larger value of eqTol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to get the stopping criterion right first and then review the comment, but strict here seems superfluous.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should be vague in the comment. It's just a parameter to protect against numerical noise at the cost of missing a minima.

// will be more robust to numeric fluctuations but will miss real purturbations
// in the objective function while a smaller value of eqTol will be more sensative
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you keep this, then sensative -> sensitive.

When you edit this, could you move the whole EqTol comment to the field in the struct? And while you are at it, do the same for GradConst?

// to numeric fluctuations but is more likely to result in ErrLinesearcherFailure.
type Bisection struct {
GradConst float64
EqTol float64

eqTol float64
minStep float64
maxStep float64
currStep float64

initF float64
minF float64
maxF float64
lastF float64

initGrad float64
absInitGrad float64

lastOp Operation
}
Expand All @@ -41,16 +54,20 @@ func (b *Bisection) Init(f, g float64, step float64) Operation {
if b.GradConst <= 0 || b.GradConst >= 1 {
panic("bisection: GradConst not between 0 and 1")
}
if b.EqTol == 0 {
b.eqTol = 1e-10
} else if b.EqTol < 0 {
b.eqTol = 0
}

b.minStep = 0
b.maxStep = math.Inf(1)
b.currStep = step

b.initF = f
b.minF = f
b.maxF = math.NaN()

b.initGrad = g
b.absInitGrad = math.Abs(g)

// Only evaluate the gradient when necessary.
b.lastOp = FuncEvaluation
Expand All @@ -61,18 +78,15 @@ func (b *Bisection) Iterate(f, g float64) (Operation, float64, error) {
if b.lastOp != FuncEvaluation && b.lastOp != GradEvaluation {
panic("bisection: Init has not been called")
}
minF := b.initF
minF := b.minF
if b.maxF < minF {
minF = b.maxF
}
if b.minF < minF {
minF = b.minF
}
if b.lastOp == FuncEvaluation {
// See if the function value is good enough to make progress. If it is,
// evaluate the gradient. If not, set it to the upper bound if the bound
// has not yet been found, otherwise iterate toward the minimum location.
if f <= b.minF {
if f <= b.minF || floats.EqualWithinAbsOrRel(f, minF, b.eqTol, b.eqTol) {
b.lastF = f
b.lastOp = GradEvaluation
return b.lastOp, b.currStep, nil
Expand All @@ -94,7 +108,7 @@ func (b *Bisection) Iterate(f, g float64) (Operation, float64, error) {
f = b.lastF
// The function value was lower. Check if this location is sufficient to
// converge the linesearch, otherwise iterate.
if StrongWolfeConditionsMet(f, g, minF, b.initGrad, b.currStep, 0, b.GradConst) {
if math.Abs(g) < b.GradConst*b.absInitGrad {
b.lastOp = MajorIteration
return b.lastOp, b.currStep, nil
}
Expand Down