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

added COBYLA (no REST interface yet) #392

Merged
merged 12 commits into from
Nov 10, 2014
Merged

Conversation

denizokt
Copy link
Contributor

********* PEOPLE *************
Primary reviewer: @sc932

Reviewers: @suntzu86

********* DESCRIPTION **************
Branch Name: denizokt_370_constraints
Ticket(s)/Issue(s): Closes #370

********* TESTING DONE *************
make test
make style-test

: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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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.


r"""Optimizes an objective function over the specified contraints with the COBYLA method.

.. Note:: See optimize() docstring for more details.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest link?

:doc: or :class: or whatever it is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:func: actually.

:func:`moe.whatever.path.to.file.class.optimize`

you can also do :mod: for modules (files) or :class: for classes. Put a tilde like:

:func:`~moe.whatever.path.to.file.class.optimize`

if you odnt want the link to spell out the whole name (usually only applicable when you're referencing class.foo() from class.bar(), something nearby)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

func should be of the form compute_* in interfaces.optimization_interface.OptimizableInterface.
"""
def decorated(point):
"""Decorator for compute_* functions in interfaces.optimization_interface.OptimizableInterface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compute_* with backtics for good formatting

rst link here too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this what you mean?

@sc932
Copy link
Contributor

sc932 commented Aug 22, 2014

Looks mostly good. Needs a lot more comments.

@suntzu86
Copy link
Contributor

thanks for cleaning up the optimizer testing code! i'll revisit this more later

edit: left you one more comment about generalizing scipy wrappers

@suntzu86
Copy link
Contributor

also please update the changelog

@@ -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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to have:

``compute_*``

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.

@@ -32,6 +32,11 @@ def get_bounding_box(self):
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise(NotImplementedError, "reason...")

Never fail silently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an abstract function though

@sc932
Copy link
Contributor

sc932 commented Aug 22, 2014

I'm happy :)

@suntzu86 are you good to push and ticket anything that comes up?

:param optimization_parameters: parameters describing how to perform optimization (tolerances, iterations, etc.)
:type optimization_parameters: python_version.optimization.LBFGSBParameters object
:type optimization_parameters: ``python_version.optimization.*Parameters`` object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd be more explicit about the parameter struct needing to match what's called in: get_scipy_optimizer_unshaped_point

this is part of why i wanted the scipy optimizer call to be settable in the ctor--then you can more easily be explicit about what needs to match what. And you don't have 2 optimization functions.

Deniz Oktay and others added 4 commits August 22, 2014 16:32
Merge branch 'master' of github.com:yelp/MOE into denizokt_370_constraints

Conflicts:
	moe/tests/optimal_learning/python/python_version/optimization_test.py
@sc932
Copy link
Contributor

sc932 commented Nov 10, 2014

🐑 it

suntzu86 added a commit that referenced this pull request Nov 10, 2014
added COBYLA (no REST interface yet)
@suntzu86 suntzu86 merged commit 53318ab into master Nov 10, 2014
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.

implement constrained optimization
3 participants