-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add f_calls_limit parameter to minimise #49
base: master
Are you sure you want to change the base?
add f_calls_limit parameter to minimise #49
Conversation
Codecov Report
@@ Coverage Diff @@
## master #49 +/- ##
==========================================
+ Coverage 89.55% 90.14% +0.58%
==========================================
Files 4 4
Lines 67 71 +4
==========================================
+ Hits 60 64 +4
Misses 7 7
Continue to review full report at Codecov.
|
src/optimise.jl
Outdated
@@ -38,6 +39,7 @@ function minimise(f, X::T; structure = HeapedVector, tol=1e-3) where {T} | |||
|
|||
# find candidate for upper bound of global minimum by just evaluating a point in the interval: | |||
m = sup(f(Interval.(mid.(X)))) # evaluate at midpoint of current interval | |||
f_calls += 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a single call to f so should just increment by 1?
Great, thanks! Could you please add a couple of tests for this? |
I've added a test and improved a bit the termination criteria so it's more accurate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the delay in getting to this. I like the idea; I just have a couple of comments about the implementation.
@@ -2,11 +2,11 @@ | |||
numeric_type(::IntervalBox{N, T}) where {N, T} = T | |||
|
|||
""" | |||
minimise(f, X, structure = SortedVector, tol=1e-3) | |||
minimise(f, X, structure = SortedVector, tol=1e-3, f_calls_limit=Inf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inf
is a Float64
. Presumably the number of calls should be an integer. So this should be replaced by some large integer like 10^20
, or given a sentinel value like -1
(probably preferable).
@@ -35,6 +35,11 @@ using IntervalOptimisation: numeric_type | |||
@test length(minimisers) == 1 | |||
@test minimisers[1] ⊆ -0.1..0.1 | |||
|
|||
global n_calls = 0 | |||
f = x -> begin global n_calls += 1; x^2 end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably use a let
block instead of the global variable.
global n_calls = 0 | ||
f = x -> begin global n_calls += 1; x^2 end | ||
global_min, minimisers = minimise(f, -10..11, tol = 1e-10, f_calls_limit = 10) | ||
@test n_calls == 10 #note : for other values we can overshoot, since we call 3 times per iteration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the minimise
function should return the total number of calls.
This adds an option to limit the number of function evaluations the minimise method can do. I used the same name as in Optim.jl.
Using the following I counted that the function was getting called 3 times per iteration, which seems to hold across dimensions.
fix #44