-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add support to use ipopt to solve constrained problem #722
Conversation
src/Drivers/hiopbbpy/BODriverEX.py
Outdated
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.
Is the default solver in ipopt a quasi-Newton method when Hessian information is not provided? If the default option is quasi-Newton then the infastructure seems ready for problems with nonlinear constraints. If the default option is not quasi-Newton then we should make it our default option in optproblem.py
.
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 had checked it, and ipopt uses quasi-newton since Hessian is not provided.
If I set option hessian_approximation
to exact
, ipopt will throw errors and terminates.
yes, supporting nonlinear constraints is not hard, but we also need to change the way scipy loads the problem.
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.
Well good. It seems we don't want to use full-Newton methods at the moment. I guess we can talk about the problem of linear vs nonlinear constraints at a later time.
src/hiopbbpy/opt/optproblem.py
Outdated
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.
Since here, the constraints are either c = 0
or c >= 0
, it might be worth noting that here (or somewhere else) in the comments.
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 looks good to 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.
lgtm
one other comment would be related to name IpoptProbFromScipy
. I think just IpoptProb
would do it for now.
finally, user_constraints
are attached to BOAlgorithm. Wouldn't make more sense to be attached to the problem
instead?
CI is fixed! |
@cnpetra @thartland I just push a commit, in which:
|
3d53d56
to
594b756
Compare
I do not see the same mismatch you are seeing. I the result of |
It is reasonable to terminate the BO loop if the optimizer for the acquisition function fails to find a single local minimizer from the multiple starting points. |
I figured this out. I should not use -1 as the default value, as it indicates maximum iterations exceed. I will change it to -999 in the upcoming commit. |
Right now CI fails as expected, since ipopt is not installed in the container.
I cannot find preinstalled ipopt from the github maketplace, and hence I'd like to swith to conda and install cyipopt and ipopt under conda.