Conversation
Codecov Report
@@ Coverage Diff @@
## master #9 +/- ##
==========================================
- Coverage 81.08% 24.78% -56.3%
==========================================
Files 3 3
Lines 37 117 +80
==========================================
- Hits 30 29 -1
- Misses 7 88 +81
Continue to review full report at Codecov.
|
src/optimise1.jl
Outdated
|
|
||
| import Base.isless | ||
|
|
||
| struct IntervalMinima{T<:Real} |
There was a problem hiding this comment.
"Minimum" is the singular; "minima" is the plural.
There was a problem hiding this comment.
You want "minimum" everywhere.
There was a problem hiding this comment.
Okay. Using global_minimum instead since minimum is an existent binding.
src/optimise1.jl
Outdated
| Q = binary_minheap(IntervalMinima{T}) | ||
|
|
||
| minima = f(interval(mid(x))).hi | ||
| arg_minima = Interval{T}[] |
There was a problem hiding this comment.
"Minima" is correct here since there may be several.
|
|
||
|
|
||
| # Unconstrained Optimisation | ||
| f(x, y) = (1.5 - x * (1 - y))^2 + (2.25 - x * (1 - y^2))^2 + (2.625 - x * (1 - y^3))^2 |
There was a problem hiding this comment.
Please include the source for test cases and solutions.
|
There are some test cases here: |
|
See also the links at |
src/optimise1.jl
Outdated
| return lb..global_minimum, arg_minima | ||
| end | ||
|
|
||
| function minimise1d_deriv(f::Function, x::Interval{T}; reltol=1e-3, abstol=1e-3) where {T<:Real} |
There was a problem hiding this comment.
There seems to be a lot of common code between this function and minimise1d. Can that be refactored out, or can these functions be combined?
There was a problem hiding this comment.
I'll add deriv as a kwarg and combine the functions
src/optimise1.jl
Outdated
| return lb..global_minimum, arg_minima | ||
| end | ||
|
|
||
| function minimise_icp_constrained(f::Function, x::IntervalBox{N, T}, constraints::Vector{constraint{T}} = Vector{constraint{T}}(); reltol=1e-3, abstol=1e-3) where {N, T<:Real} |
There was a problem hiding this comment.
There seems to be a lot of common code between this function and the previous one. Can that be refactored out, or can these functions be combined?
There was a problem hiding this comment.
I added them differently because the GODOWN used for them is different, as we can use the mid in the unconstrained case and that causes better convergence, whereas it's not possible in the constrained case since mid might not be satisfied by the constraints.
Also, the robust constrained optimisation function will be different and will include John conditions, so I kept them separate for now.
Should I combine them using runtime checks?
src/optimise1.jl
Outdated
| # if 0 .∉ ForwardDiff.gradient(f, p.intval.v) | ||
| # continue | ||
| # end | ||
| X = icp(f, p.intval, -∞..global_minimum) |
src/optimise1.jl
Outdated
| import Base.isless | ||
|
|
||
| struct IntervalMinima{T<:Real} | ||
| intval::Interval{T} |
There was a problem hiding this comment.
I would prefer to write out interval
|
Please add more test cases. |
src/optimise1.jl
Outdated
|
|
||
| import Base.isless | ||
|
|
||
| struct IntervalMinima{T<:Real} |
src/optimise1.jl
Outdated
|
|
||
| struct IntervalMinima{T<:Real} | ||
| intval::Interval{T} | ||
| global_minimum::T |
src/optimise1.jl
Outdated
| global_minimum::T | ||
| end | ||
|
|
||
| function isless(a::IntervalMinima{T}, b::IntervalMinima{T}) where {T<:Real} |
There was a problem hiding this comment.
You can write this as a single-line function.
src/optimise1.jl
Outdated
| continue | ||
| end | ||
|
|
||
| deriv = ForwardDiff.derivative(f, p.intval) |
There was a problem hiding this comment.
It should be optional to use this.
src/optimise1.jl
Outdated
| continue | ||
| end | ||
| # Second derivative contractor | ||
| # doublederiv = ForwardDiff.derivative(x->ForwardDiff.derivative(f, x), p.intval) |
There was a problem hiding this comment.
It should be optional to use this. Is there a way of setting up the 2nd derivative beforehand to reduce the workload?
There was a problem hiding this comment.
Okay.
I removed it because the second deriv evaluation is costly, and since the interval to be computed over is different for each iteration, it can slow down the function.
Also, it didn't show any improvements over the first deriv contractor
src/optimise1.jl
Outdated
|
|
||
|
|
||
|
|
||
| struct IntervalBoxMinima{N, T<:Real} |
src/optimise1.jl
Outdated
|
|
||
|
|
||
| struct IntervalBoxMinima{N, T<:Real} | ||
| intval::IntervalBox{N, T} |
There was a problem hiding this comment.
You don't need a separate type here -- just reuse IntervalMinimum.
| end | ||
|
|
||
| struct constraint{T<:Real} | ||
| f::Function |
There was a problem hiding this comment.
This should be a type parameter of the Constraint type for efficiency.
src/optimise1.jl
Outdated
| global_minimum::T | ||
| end | ||
|
|
||
| struct constraint{T<:Real} |
src/optimise1.jl
Outdated
| global_minimum::T | ||
| end | ||
|
|
||
| struct constraint{T<:Real} |
|
I think that |
|
|
||
| Base.isless(a::IntervalBoxMinimum{N, T}, b::IntervalBoxMinimum{N, T}) where {N, T<:Real} = isless(a.minimum, b.minimum) | ||
|
|
||
| function minimise_icp(f::Function, x::IntervalBox{N, T}; reltol=1e-3, abstol=1e-3) where {N, T<:Real} |
There was a problem hiding this comment.
Make this a method of minimise and dispatch on a type ICP or similar.
| return lb..global_minimum, arg_minima | ||
| end | ||
|
|
||
| function minimise_icp_constrained(f::Function, x::IntervalBox{N, T}, constraints::Vector{Constraint{T}} = Vector{Constraint{T}}(); reltol=1e-3, abstol=1e-3) where {N, T<:Real} |
There was a problem hiding this comment.
Make this a method of minimise and dispatch on a type.
No description provided.