Skip to content
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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathanBieler
Copy link

@jonathanBieler jonathanBieler commented Dec 17, 2019

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.

i=0; minimise( x -> begin global i=i+1; @show i; sum(abs2.(x)) end, IntervalBox(-600..600, 5); tol=1e-6, f_calls_limit=20 )

fix #44

@codecov-io
Copy link

codecov-io commented Dec 17, 2019

Codecov Report

Merging #49 into master will increase coverage by 0.58%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
src/optimise.jl 96% <100%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3476f34...31712e4. Read the comment docs.

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
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 a single call to f so should just increment by 1?

@dpsanders
Copy link
Member

Great, thanks! Could you please add a couple of tests for this?

@jonathanBieler
Copy link
Author

I've added a test and improved a bit the termination criteria so it's more accurate.

Copy link
Member

@dpsanders dpsanders left a 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)
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add maximum number of function evaluation ?
3 participants