-
Notifications
You must be signed in to change notification settings - Fork 45
Nevergrad: OnePlusOne Optimiser addition #576
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
base: main
Are you sure you want to change the base?
Conversation
@janosg
|
def _solve_internal_problem( | ||
self, problem: InternalOptimizationProblem, x0: NDArray[np.float64] | ||
) -> InternalOptimizeResult: | ||
print(problem.bounds) |
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.
Here I have tried to print bound
This does not contain enough information to understand your problem. We need at least the following:
Ideally you follow this blogpost when describing problems you encounter.
What is your question or doubt? If I understand correctly, this is how most global optimizers behave.
Algorithms in optimagic can have as many options as you need and they can have any type. So it should not be a problem to allow complete configurability of the algorithm. |
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Hi @janosg, I've corrected the error from my previous message. I've now added the OnePlusOne wrapper, and it's ready for your review. There's a setting to control the maximum optimization time in Nevergrad – any ideas for a more descriptive name? I'm still working on adding constraints. |
Hi @gulshan-123, thanks for the PR. I'll do a thorough review once your wrapper is feature complete. So here are just some quick comments to help you get there:
The documentation is an essential part of the PR and needs to convince us that you did a thorough job in exploring all the tuning parameters of the algorithm. |
OnePlusOne, as implemented in Nevergrad, is a variant of the (1+1)-Evolution Strategy. It operates by maintaining a single current solution
I will try this. |
Hi @janosg , I have added all the parameters from nevergrad OnePlusOne in optimagic. |
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.
Hi @gulshan-123. Sorry for the late review.
Besides the specific comments we need the following changes:
- Install nevergrad via pip instead of conda to fix the test failures (the conda version is outdated)
- Solve the merge conflicts with the current main branch (In the meantime, we have merged the first PR that adds another nevergrad optimizer, so there are a few conflicts).
c64aa9f
to
7c9e0ab
Compare
Hello @janosg, I first pulled the latest main branch and then made changes on that only to avoid any merge conflict later. The following test is failing. ____________________________________________________________ test_algorithm_on_sum_of_squares_with_binding_bounds[nevergrad_oneplusone] _____________________________________________________________
algorithm = 'nevergrad_oneplusone'
@pytest.mark.parametrize("algorithm", BOUNDED_ALGORITHMS)
def test_algorithm_on_sum_of_squares_with_binding_bounds(algorithm):
res = minimize(
fun=sos,
params=np.array([3, 2, -3]),
bounds=Bounds(
lower=np.array([1, -np.inf, -np.inf]), upper=np.array([np.inf, np.inf, -1])
),
algorithm=algorithm,
collect_history=True,
skip_checks=True,
)
assert res.success in [True, None]
decimal = 3
> aaae(res.params, np.array([1, 0, -1]), decimal=decimal)
E AssertionError:
E Arrays are not almost equal to 3 decimals
E
E Mismatched elements: 1 / 3 (33.3%)
E Max absolute difference among violations: 0.32302791
E Max relative difference among violations: inf
E ACTUAL: array([ 1. , 0.323, -1. ])
E DESIRED: array([ 1, 0, -1]) As discussed here #579 (comment), I tried to change the default parameters, but was still failing. Even after increasing the budget, |
Hi @gauravmanmode, i have added the docs and all the pytests are passing. Thank you very much. |
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.
Hi @gulshan-123, the code looks good. I made a few comments in the documentation. We can merge after they are addressed.
added docs Docs success build updated docs done with suggestions
849fba9
to
4fd75fe
Compare
@janosg, I have changed as suggested. Tests are still failing due to answer coming close to 0 instead of exact 0. |
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
@gulshan-123 the tests are passing now. The key was to mark the algorithm as global. The precision requirements for global algorithms are lower than for local ones. |
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.
I made a few more comments about more specific type hints and clearer documentation.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Hi @janosg, I have fixed some things and moved the documentation to docstring. |
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.
@gauravmanmode, thanks for taking over! A few tiny comments but I already approve. You can fix the comments and then merge without another review.
The pre-commit failure is a general thing affecting all PRs currently. @timmens will look for a fix soon. |
I have written a basic structure of OnePlusOne optimisation wrapper.
Partially fixes #560