ENH: optimize: add optional parameters of adaptive step size adjustment in `bashinhopping`. #14941

AtsushiSakai · 2021-10-30T14:30:23Z

Reference issue

What does this implement/fix?

optimize.bashinhopping has adaptive step size adjustment using this class.

Lines 181 to 205 in 0f91f0f

    
           class AdaptiveStepsize: 
        
               """ 
        
               Class to implement adaptive stepsize. 
        
               This class wraps the step taking class and modifies the stepsize to 
        
               ensure the true acceptance rate is as close as possible to the target. 
        
               Parameters 
        
               ---------- 
        
               takestep : callable 
        
                   The step taking routine.  Must contain modifiable attribute 
        
                   takestep.stepsize 
        
               accept_rate : float, optional 
        
                   The target step acceptance rate 
        
               interval : int, optional 
        
                   Interval for how often to update the stepsize 
        
               factor : float, optional 
        
                   The step size is multiplied or divided by this factor upon each 
        
                   update. 
        
               verbose : bool, optional 
        
                   Print information about each update 
        
               """ 
        
               def __init__(self, takestep, accept_rate=0.5, interval=50, factor=0.9, 
        
                            verbose=True):

The AdaptiveStepsize class has optional parameters accept_rate and factor to configure the behavior of adaptive step size adjustment.
However, scipy users cannot configure these parameters because bashinhopping does not have arguments to set these.
bashinhopping only provides an argument to set intervel.
So, I added the optional parameters accept_rate and factor for bashinhopping.

…nt in `bashinhopping`.

tupui

Thanks @AtsushiSakai. Seems like it was missing, but maybe there was a reason behind this 🤷‍♂️. @andyfaff is this ok to add these? Or should we just fix these as constants?

I can see there are no tests for these 2 parameters in general.

scipy/optimize/_basinhopping.py

Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>

andyfaff

It looks good to me. Are there sensible ranges for the newly added keywords? If so, then they should be outlined, otherwise how is the user supposed to know how to change them for different performance?

andyfaff · 2021-11-10T05:33:55Z

scipy/optimize/_basinhopping.py

+
+        .. versionadded:: 1.8.0
+
+    factor : float, optional


What is the sensible value range for factor??

0 and < 1.

It might make sense to raise an exception if it is out of that range.

andyfaff · 2021-11-10T05:34:48Z

scipy/optimize/_basinhopping.py

@@ -437,6 +437,19 @@ def basinhopping(func, x0, niter=100, T=1.0, stepsize=0.5,
        `take_step` and `accept_test`, and these functions use random
        number generation, then those functions are responsible for the state
        of their random number generator.
+    accept_rate : float, optional


What is the sensible value range for accept_rate?

> 0 and < 1. It would probably make sense to raise an exception otherwise.

Thanks and good point about the validation since this is going to be exposed. Same for factor.

tupui · 2021-11-10T07:59:41Z

@js850 do you recall why you set fix values for these 2 parameters? Is this empirical or from some literature?
cc @dlax since you reviewed this as well. I did not find anything in the original PR introducing basinhopping.

js850 · 2021-11-10T10:54:32Z

In my original PR there was some discussion about how many parameters to expose, i.e. the tension between simplicity for the new user and flexibility for the power user.

It's been some time since I was immersed in this topic, but if I recall correctly, both of these are adjustable parameters, i.e. there is no theory saying what the right value is. If there is demand for exposing them as publicly adjustable parameters then have no reason to oppose it.

js850 · 2021-11-10T10:20:28Z

scipy/optimize/_basinhopping.py

+
+        .. versionadded:: 1.8.0
+
+    factor : float, optional


0 and < 1.

It might make sense to raise an exception if it is out of that range.

js850 · 2021-11-10T10:20:58Z

scipy/optimize/_basinhopping.py

@@ -437,6 +437,19 @@ def basinhopping(func, x0, niter=100, T=1.0, stepsize=0.5,
        `take_step` and `accept_test`, and these functions use random
        number generation, then those functions are responsible for the state
        of their random number generator.
+    accept_rate : float, optional


> 0 and < 1. It would probably make sense to raise an exception otherwise.

js850 · 2021-11-10T10:22:50Z

scipy/optimize/_basinhopping.py

+
+        .. versionadded:: 1.8.0
+
+    factor : float, optional


You might consider giving it a more self-explanatory name. e.g. stepsize_adjustment_factor. But then that is quite long, so I will leave it up to you.

At most I would go with stepsize_factor or it would be long as you said.

I think stepsize_factor works here.

js850 · 2021-11-10T10:23:58Z

scipy/optimize/_basinhopping.py

@@ -437,6 +437,19 @@ def basinhopping(func, x0, niter=100, T=1.0, stepsize=0.5,
        `take_step` and `accept_test`, and these functions use random
        number generation, then those functions are responsible for the state
        of their random number generator.
+    accept_rate : float, optional


target_accept_rate would be more self-explanatory, but it is also longer, so whatever you feel is better is fine.

Isn't target already implied?

I think target_accept_rate would be preferred here.

tupui

It's been some time since I was immersed in this topic, but if I recall correctly, both of these are adjustable parameters, i.e. there is no theory saying what the right value is.

Thanks @js850 for the information and review. It's fine if we don't have more knowledge on these for now. Interested people might ask to improve the doc once this has been used or if some already know.

tupui · 2021-11-10T10:56:31Z

scipy/optimize/_basinhopping.py

@@ -437,6 +437,19 @@ def basinhopping(func, x0, niter=100, T=1.0, stepsize=0.5,
        `take_step` and `accept_test`, and these functions use random
        number generation, then those functions are responsible for the state
        of their random number generator.
+    accept_rate : float, optional


Thanks and good point about the validation since this is going to be exposed. Same for factor.

tupui · 2021-11-10T10:58:42Z

scipy/optimize/_basinhopping.py

+
+        .. versionadded:: 1.8.0
+
+    factor : float, optional


At most I would go with stepsize_factor or it would be long as you said.

tupui · 2021-11-10T10:59:27Z

scipy/optimize/_basinhopping.py

@@ -437,6 +437,19 @@ def basinhopping(func, x0, niter=100, T=1.0, stepsize=0.5,
        `take_step` and `accept_test`, and these functions use random
        number generation, then those functions are responsible for the state
        of their random number generator.
+    accept_rate : float, optional


Isn't target already implied?

andyfaff · 2021-11-10T22:43:25Z

To summarise:

change factor to stepwise_factor
change accept_rate to target_accept_rate
Document acceptable ranges for both those parameters. target_accept_rate and stepwise_factor should be greater than zero and less than 1.

tupui · 2021-11-11T07:30:15Z

Add input validation and corresponding tests

…n, and test it.

tupui

LGTM, thanks @AtsushiSakai and everyone for the help reviewing.

ENH: optimize: add optional parameters of adaptive step size adjustme…

d85cabe

…nt in `bashinhopping`.

AtsushiSakai requested a review from andyfaff as a code owner October 30, 2021 14:30

github-actions bot added the scipy.optimize label Oct 30, 2021

AtsushiSakai added the enhancement A new feature or improvement label Oct 30, 2021

tupui reviewed Nov 3, 2021

View reviewed changes

scipy/optimize/_basinhopping.py Outdated Show resolved Hide resolved

scipy/optimize/_basinhopping.py Outdated Show resolved Hide resolved

AtsushiSakai and others added 2 commits November 3, 2021 21:40

ENH: optimize: improve docs

68c02f5

Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>

ENH: optimize: improve signiture

d2a3b80

Co-authored-by: Pamphile ROY <roy.pamphile@gmail.com>

andyfaff reviewed Nov 10, 2021

View reviewed changes

andyfaff mentioned this pull request Nov 10, 2021

ENH: added accept_rate as a keyword to basinhopping #8775

Closed

js850 reviewed Nov 10, 2021

View reviewed changes

tupui reviewed Nov 10, 2021

View reviewed changes

ENH: optimize: change arguments name, update doc, add input validatio…

de34d65

…n, and test it.

tupui approved these changes Nov 11, 2021

View reviewed changes

tupui merged commit fa58cce into scipy:master Nov 11, 2021

tupui added this to the 1.8.0 milestone Nov 11, 2021

AtsushiSakai deleted the issue_8774 branch November 11, 2021 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: optimize: add optional parameters of adaptive step size adjustment in `bashinhopping`. #14941

ENH: optimize: add optional parameters of adaptive step size adjustment in `bashinhopping`. #14941

AtsushiSakai commented Oct 30, 2021

tupui left a comment

andyfaff left a comment

andyfaff Nov 10, 2021

js850 Nov 10, 2021

andyfaff Nov 10, 2021

js850 Nov 10, 2021

tupui Nov 10, 2021

tupui commented Nov 10, 2021 •

edited

Loading

js850 commented Nov 10, 2021

js850 Nov 10, 2021

js850 Nov 10, 2021

js850 Nov 10, 2021

tupui Nov 10, 2021

andyfaff Nov 10, 2021

js850 Nov 10, 2021

tupui Nov 10, 2021

andyfaff Nov 10, 2021

tupui left a comment

tupui Nov 10, 2021

tupui Nov 10, 2021

tupui Nov 10, 2021

andyfaff commented Nov 10, 2021

tupui commented Nov 11, 2021

tupui left a comment

	class AdaptiveStepsize:
	"""
	Class to implement adaptive stepsize.

	This class wraps the step taking class and modifies the stepsize to
	ensure the true acceptance rate is as close as possible to the target.

	Parameters
	----------
	takestep : callable
	The step taking routine. Must contain modifiable attribute
	takestep.stepsize
	accept_rate : float, optional
	The target step acceptance rate
	interval : int, optional
	Interval for how often to update the stepsize
	factor : float, optional
	The step size is multiplied or divided by this factor upon each
	update.
	verbose : bool, optional
	Print information about each update

	"""
	def __init__(self, takestep, accept_rate=0.5, interval=50, factor=0.9,
	verbose=True):

ENH: optimize: add optional parameters of adaptive step size adjustment in bashinhopping. #14941

ENH: optimize: add optional parameters of adaptive step size adjustment in bashinhopping. #14941

Conversation

AtsushiSakai commented Oct 30, 2021

Reference issue

What does this implement/fix?

tupui left a comment

Choose a reason for hiding this comment

andyfaff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tupui commented Nov 10, 2021 • edited Loading

js850 commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tupui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyfaff commented Nov 10, 2021

tupui commented Nov 11, 2021

tupui left a comment

Choose a reason for hiding this comment

ENH: optimize: add optional parameters of adaptive step size adjustment in `bashinhopping`. #14941

ENH: optimize: add optional parameters of adaptive step size adjustment in `bashinhopping`. #14941

tupui commented Nov 10, 2021 •

edited

Loading