-
Notifications
You must be signed in to change notification settings - Fork 44
Add wrapper for Nevergrad PSO optimizer #579
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
Hello @janosg Is there anything left to implement in this PR? I would appreciate it if the CI gets approval to begin. |
@r3kste , I am also working on similar PR: about the addition of OnePlusOne algorithm from nevergrad. I am curious if we can also add non-linear constraint in wrapper as suggested here: https://facebookresearch.github.io/nevergrad/optimization.html#optimization-with-constraints, can you please look into this? (Currently, I have end semester exams so a little busy.) |
Hi @r3kste, I had not looked at your PR yet because you have not requested a review (see contributor guide). I approved the CI and will look at it as soon as possible. |
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 @r3kste, thanks for the PR. The changes look really good. I just made a few minor stylistic comments.
Currently the tests are not passing but I think this is not related to a problem in your code. It rather seems like nevergrad has a few things that are not yet compatible with numpy 2.0.
Can you pin numpy to <2.0 in the environment to confirm this? If it works we can see how to deal with the situation.
num_workers=self.n_cores, | ||
) | ||
|
||
while optimizer.num_ask < optimizer.budget: |
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.
Minor thing, but I think it would be more readable to see while optimizer.num_ask <= self.stopping_maxfun
result = InternalOptimizeResult( | ||
x=recommendation.value[0][0], | ||
fun=recommendation.loss, | ||
success=True, | ||
n_fun_evals=optimizer.num_ask, |
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.
Can you try to define a few more fields of the InternalOptimizeResult
, even if they are trivial (e.g. n_jac_evals
could be 0 instead of None).
x_list = [ | ||
optimizer.ask() | ||
for _ in range( | ||
min(optimizer.num_workers, optimizer.budget - optimizer.num_ask) |
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.
Similarly, I would prefer self.n_cores
over optimizer.num_workers
here. For readers who are familiar with optimagic but not nevergrad (which is going to be the majority) it is easier to understand the optimagic terminology than the nevergrad terminology.
Codecov ReportAttention: Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Hello, Thanks for the review. I have committed the suggested changes. I want to mention that there are a few errors raised by mypy in pre-commit, but I believe these are unrelated to the PR. src/optimagic/optimization/history.py:412: error: Unused "type: ignore" comment [unused-ignore]
src/optimagic/optimization/history.py:533: error: Unused "type: ignore" comment [unused-ignore]
src/optimagic/logging/logger.py:193: error: Unused "type: ignore" comment [unused-ignore]
Found 3 errors in 2 files (checked 1 source file)
This was caused because |
Implements the Particle Swarm Optimization algorithm from
nevergrad
.Partially fixes #560