-
Notifications
You must be signed in to change notification settings - Fork 140
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
added COBYLA (no REST interface yet) #392
Changes from 5 commits
e3471da
4d2ad53
f4020a9
3fc8bb7
c38a678
07c214c
ea279ee
a79da85
05bf53c
2b877c5
506af3e
ec5f660
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,11 @@ def get_bounding_box(self): | |
"""Return a list of ClosedIntervals representing a bounding box for this domain.""" | ||
pass | ||
|
||
@abstractmethod | ||
def get_constraint_list(self, start_index=0): | ||
"""Return a list of lambda functions expressing the domain bounds as linear constraints. Used by COBYLA.""" | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. raise(NotImplementedError, "reason...") Never fail silently. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is an abstract function though |
||
|
||
@abstractmethod | ||
def generate_random_point_in_domain(self, random_source=None): | ||
"""Generate ``point`` uniformly at random such that ``self.check_point_inside(point)`` is True. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,6 +85,25 @@ def get_bounding_box(self): | |
"""Return a list of ClosedIntervals representing a bounding box for this domain.""" | ||
return copy.copy(self._domain_bounds) | ||
|
||
def get_constraint_list(self, start_index=0): | ||
"""Return a list of lambda functions expressing the domain bounds as linear constraints. Used by COBYLA. | ||
|
||
Since COBYLA in scipy only optimizes arrays, we flatten out our points while doing multipoint EI optimization. | ||
But in order for the constraints to access the correct index, the RepeatedDomain class has to signal which index | ||
the TensorProductDomain should start from, using the start_index optional parameter. | ||
|
||
:param start_index: the dimension this tensor product domain should start indexing from | ||
:type start_index: integer (>= 0) | ||
:return: a list of lambda functions corresponding to constraints | ||
:rtype: array of lambda functions with shape (dim * 2) | ||
|
||
""" | ||
constraints = [] | ||
for i, interval in enumerate(self._domain_bounds): | ||
constraints.append((lambda x: x[i + start_index] - interval.min)) | ||
constraints.append((lambda x: interval.max - x[i + start_index])) | ||
return constraints | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is severely under-documented.... why do you need start_index? Also the scipy docs make it sound like x >= 0 is required: Constraint functions; must all be >=0 (a single function if only 1 constraint). Each function takes the parameters x as its first argument. also have you checked that these constraints give you what you want? how does "lambda" capture local variables in python? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so when we are doing multi-point EI optimization, we flatten out the points such that it is a single long array and then feed it into the optimizer. therefore we end up having a repeateddomain over a bunch of tensor products with the same dimension. but since we want the constraints to only access the specific index in the flattened array, i created a "start_index" so the repeated_domain class can signal which array index the tensordomain class should start from. does this make sense? they do make it sound like that, but i'm pretty sure it means that x >= 0 iff constraint is satisfied. the examples they give seem to support that. hmm yeah i think this does what i want. at least... i'm pretty sure it works correctly? lol There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is pretty non-intuitive and could definitely use some explaining in docstrings. i think it'd also be possible to access I see what you mean about the x >= 0 thing. i think you're right. Have you tested that the constrained part works (i can't remember if the basic optimization unit tests have constrained cases) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah the constrained part works, the optimization unit tests do use constraints |
||
|
||
def generate_random_point_in_domain(self, random_source=None): | ||
"""Generate ``point`` uniformly at random such that ``self.check_point_inside(point)`` is True. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -324,6 +324,32 @@ class LBFGSBParameters(_BaseLBFGSBParameters): | |
__slots__ = () | ||
|
||
|
||
# See ConstrainedDFOParameters (below) for docstring. | ||
_BaseConstrainedDFOParameters = collections.namedtuple('_BaseConstrainedDFOParameters', [ | ||
'rhobeg', | ||
'rhoend', | ||
'maxfun', | ||
'catol', | ||
]) | ||
|
||
|
||
class ConstrainedDFOParameters(_BaseConstrainedDFOParameters): | ||
|
||
r"""Container to hold parameters that specify the behavior of COBYLA. | ||
|
||
Suggested values come from scipy documentation for scipy.optimize.fmin_cobyla: | ||
http://docs.scipy.org/doc/scipy-0.13.0/reference/generated/scipy.optimize.fmin_cobyla.html | ||
|
||
:ivar rhobeg: (*float64 > 0.0*) reasonable initial changes to the variables (suggest: 1.0) | ||
:ivar rhoend: (*float64 > 0.0*) final accuracy in the optimization (not precisely guaranteed), which is a lower bound on the size of the trust region (suggest: 1.0e-4) | ||
:ivar maxfun: (*int > 0*) maximum number of objective function calls to make (suggest: 1000) | ||
:ivar catol: (*float64 > 0.0*) absolute tolerance for constraint violations (suggest: 2.0e-4) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you have any intuition from testing or guides to link to or whatever describing what this stuff does? where did your suggested defaults come from? presumably rhobeg/end should depend on the length scales of the problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm i don't have too much intuition, especially for what rhobeg is. but i do know that rhoend best corresponds with the end result accuracy. the suggested values are from scipy docs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idk if there's time for this, but it'd be good to gain some understanding of how these params cause cobyla to behave with EPI problems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is fine to reference docs for now. This is an expert feature. |
||
|
||
""" | ||
|
||
__slots__ = () | ||
|
||
|
||
class NullOptimizer(OptimizerInterface): | ||
|
||
"""A "null" or identity optimizer: this does nothing. It is used to perform "dumb" search with MultistartOptimizer.""" | ||
|
@@ -599,15 +625,16 @@ def __init__(self, domain, optimizable, optimization_parameters, num_random_samp | |
def _scipy_decorator(self, func, **kwargs): | ||
"""Wrapper function for expected improvement calculation to feed into BFGS. | ||
|
||
func should be of the form compute_* in interfaces.optimization_interface.OptimizableInterface. | ||
func should be of the form `compute_*` in :class:`moe.optimal_learning.python.interfaces.optimization_interface.OptimizableInterface`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you need to have:
the star is how rst marks boldface or empahsis or something. If you have a star without a closing star, the docs will fail to render this text and complain. if you haven't, I would run "make -B docs" in the MOE root directory. You should see 70 warnings at the end. If you have more, please identify and fix them in your text. It's also good to open up the locally generated docs (navigate to the file on your computer) and check a few places to see that the links came out correctly, things look right, etc. |
||
""" | ||
def decorated(point): | ||
"""Decorator for compute_* functions in interfaces.optimization_interface.OptimizableInterface. | ||
|
||
Converts the point to proper format and sets the current point before calling the compute function. | ||
Converts the point to proper format (array with dim (self._num_points, self.domain.dim) instead of flat array) | ||
and sets the current point before calling the compute function. | ||
|
||
:param point: the point on which to do the calculation | ||
:type point: array of float64 with shape (self._num_points * self.domain.dim) | ||
:type point: array of float64 with shape (self._num_points * self.domain.dim, ) | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be a newline btwn the last line of text and triple quote There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. k |
||
shaped_point = point.reshape(self._num_points, self.domain.dim) | ||
self.objective_function.current_point = shaped_point | ||
|
@@ -646,3 +673,81 @@ def optimize(self, **kwargs): | |
else: | ||
shaped_point = unshaped_point.reshape(self._num_points, self.domain.dim) | ||
self.objective_function.current_point = shaped_point | ||
|
||
|
||
class ConstrainedDFOOptimizer(OptimizerInterface): | ||
|
||
r"""Optimizes an objective function over the specified contraints with the COBYLA method. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wikipedia/scipy links brief description of how it works There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a link to the paper(s) would be nice too as well as a link to the scipy page. this isn't a super well known method and the wiki article says basically nothing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added link to paper and scipy docs in the optimize() method docstring There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the paper? |
||
|
||
.. Note:: See :func:`~moe.optimal_learning.python.python_version.optimization.ConstrainedDFOOptimizer.optimize()` docstring for more details. | ||
|
||
""" | ||
|
||
def __init__(self, domain, optimizable, optimization_parameters, num_random_samples=None): | ||
"""Construct a ConstrainedDFOOptimizer. | ||
|
||
:param domain: the domain that this optimizer operates over | ||
:type domain: interfaces.domain_interface.DomainInterface subclass. Only supports TensorProductDomain for now. | ||
:param optimizable: object representing the objective function being optimized | ||
:type optimizable: interfaces.optimization_interface.OptimizableInterface subclass | ||
:param optimization_parameters: parameters describing how to perform optimization (tolerances, iterations, etc.) | ||
:type optimization_parameters: python_version.optimization.ConstrainedDFOParameters object | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. num_random_samples? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm i'm actually not sure what num_random_samples does... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a heuristic that is used in the C++ multistarting algorithm. if the strong optimizer fails, then we search num_random_samples random points and return the best one. it shouldn't be needed in optimizers besides multistart... although maybe it's there so that kwargs work out or something or maybe you copied from some place that does use it, idk. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's kill it if it isn't used and doc it if it is. |
||
""" | ||
self.domain = domain | ||
self.objective_function = optimizable | ||
self.optimization_parameters = optimization_parameters | ||
self._num_points = 1 | ||
# Check if this is a repeated domain, and if so set points equal to number of repeats. | ||
if hasattr(self.domain, 'num_repeats'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be worth a 1 line comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
self._num_points = self.domain.num_repeats | ||
|
||
def _scipy_decorator(self, func, **kwargs): | ||
"""Wrapper function for expected improvement calculation to feed into COBYLA. | ||
|
||
func should be of the form compute_* in :class:`moe.optimal_learning.python.interfaces.optimization_interface.OptimizableInterface`. | ||
""" | ||
def decorated(point): | ||
"""Decorator for compute_* functions in interfaces.optimization_interface.OptimizableInterface. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
rst link here too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this what you mean? |
||
|
||
Converts the point to proper format (array with dim (self._num_points, self.domain.dim) instead of flat array) | ||
and sets the current point before calling the compute function. | ||
|
||
:param point: the point on which to do the calculation | ||
:type point: array of float64 with shape (self._num_points * self.domain.dim) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is that * supposed to be a ,? or is it flat array? Usually that is still represented There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is supposed to be a flat array. i added the comma. |
||
""" | ||
shaped_point = point.reshape(self._num_points, self.domain.dim) | ||
self.objective_function.current_point = shaped_point | ||
value = -func(**kwargs) | ||
if isinstance(value, (numpy.ndarray)): | ||
return value.flatten() | ||
else: | ||
return value | ||
|
||
return decorated | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how similar is this & the optimize function to the BFGS stuff? My sense is that they're nearly identical outside of the actual call to scipy. we should create like a scipy wrapper mixin class that captures all the common pieces. Then each implementation could specify like which optimizer to call along with how to generate the arguments. then you can do:
and only need to implement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i did want to keep the optimize functions separate. if we plan on adding general linear constraints to COBYLA, the interface for this method will be changing a lot. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? the linear constraints should be handled in the input to a domain. thru the REST interface you'd need a way for users to specify constraints (linear is fairly straightforward, just sets of dim+1 length vectors. nonlinear might require string parsing or something crazier). then we have a domain class that understands how to parse those inputs and build the set of lambda functions automatically. the domain object should encapsulate the constraint stuff. I don't see why the optimizer interface would need to change at all b/c it should never need to be directly aware of that mess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Eric here. Let's keep this as general as possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the BFGS one, I'm certain you could define a base a class like:
then
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is too much work for this branch then we can punt to a ticket if needed. |
||
|
||
def optimize(self, **kwargs): | ||
"""Perform a COBYLA optimization given the parameters in optimization_parameters. | ||
|
||
For more information, visit the scipy docs page and the original paper by Powell: | ||
http://docs.scipy.org/doc/scipy-0.13.0/reference/generated/scipy.optimize.fmin_cobyla.html | ||
http://www.damtp.cam.ac.uk/user/na/NA_papers/NA2007_03.pdf | ||
|
||
objective_function.current_point will be set to the optimal point found. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what kwargs can I pass this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. currently there are no kwargs that you can pass. i've just passed them around for consistency throughout. removing would make no difference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the idea is if your objective fcn has outside parameters or whatnot, you can pass them through as kwargs and we'll just keep passing them along. creating functors is generally a better solution but scipy does this too. |
||
""" | ||
# Parameters defined above in :class:`~moe.optimal_learning.python.python_version.optimization.LBFGSBParameters` class. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong link There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. k |
||
unshaped_point = scipy.optimize.fmin_cobyla( | ||
func=self._scipy_decorator(self.objective_function.compute_objective_function, **kwargs), | ||
x0=self.objective_function.current_point.flatten(), | ||
cons=self.domain.get_constraint_list(), | ||
rhobeg=self.optimization_parameters.rhobeg, | ||
rhoend=self.optimization_parameters.rhoend, | ||
maxfun=self.optimization_parameters.maxfun, | ||
catol=self.optimization_parameters.catol, | ||
disp=0, # Suppresses output from the routine. | ||
) | ||
if self._num_points == 1: | ||
shaped_point = unshaped_point | ||
else: | ||
shaped_point = unshaped_point.reshape(self._num_points, self.domain.dim) | ||
self.objective_function.current_point = shaped_point |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,6 +79,13 @@ def get_bounding_box(self): | |
"""Return a list of ClosedIntervals representing a bounding box for this domain.""" | ||
return self._domain.get_bounding_box() | ||
|
||
def get_constraint_list(self, start_index=0): | ||
"""Return a list of lambda functions expressing the domain bounds as linear constraints. Used by COBYLA.""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more docs please. what is start_index? use
etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added docs for start_index There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still don't see this in the new branch... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return and rtype? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. k |
||
constraints = [] | ||
for i in xrange(self.num_repeats): | ||
constraints.extend(self._domain.get_constraint_list(start_index=self._domain.dim * i)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not every domain has a get_constraint_list function. At the 'top' level somewhere it's probably worth checking that domain hasattr get_constraint_list. if it doesn't, raise an exception indicating this is required for cobyla. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. currently all the implemented domains have a valid get_constraint_list function. so i added it to the domain interface. if we implement a domain that does not have one, we can throw an exception from within the method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good |
||
return constraints | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe there's a way to change the underlying constraints to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. read above, i think this is the way to do it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. commented above. probably reasonable... definitely more detailed docstring though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah added more docstrings |
||
|
||
def generate_random_point_in_domain(self, random_source=None): | ||
"""Generate ``point`` uniformly at random such that ``self.check_point_inside(point)`` is True. | ||
|
||
|
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 seems to be repeated a bunch...
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.
yeah that's for the cpp_wrappers, which are unused right now. i could remove them?
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.
it'd be better to define a utility somewhere and call it. b/c in principle you can pass a cpp_wrappers domain around and it would work. but if you want to have this fail with "not implemented" that'd be fine too
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.
Either of those work for me.
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.
What do we want to go with (here? not implemented? utility?)? I don't like how often this is repeated. Where are the start_index docs?