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

Add support to use ipopt to solve constrained problem #722

Merged
merged 5 commits into from
Apr 10, 2025

Conversation

nychiang
Copy link
Collaborator

@nychiang nychiang commented Apr 8, 2025

  • Support constrained optimization problems --- linear constraint only!
  • Compute 1st order information for EI and LCB acquisition functions.
  • Pass 1st order information to scipy optimization solver.
  • Add support to IPOPT.
  • Provide new examples.
  • Redesign some classes

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.

Copy link
Collaborator

@thartland thartland Apr 8, 2025

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator

@thartland thartland left a 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.

Copy link
Collaborator

@cnpetra cnpetra left a 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?

@nychiang nychiang marked this pull request as ready for review April 9, 2025 21:39
@nychiang
Copy link
Collaborator Author

nychiang commented Apr 9, 2025

CI is fixed!

@nychiang
Copy link
Collaborator Author

nychiang commented Apr 9, 2025

@cnpetra @thartland I just push a commit, in which:

  1. I hide cyipopt in our class IpoptProb
  2. check Ipopt return value and msg --- currently there is a mismatch between the return value and return msg. It might caused by the version of Ipopt, and I just commented in a relevant issue in cyipopt.
  3. terminate hiop if Ipopt failes to find one solution from all the initial points.

redisgn IpoptProb
@nychiang nychiang force-pushed the hiopbbpy-v0.04-dev branch from 3d53d56 to 594b756 Compare April 9, 2025 23:34
@thartland
Copy link
Collaborator

@nychiang

@cnpetra @thartland I just push a commit, in which:

1. I hide cyipopt in our class `IpoptProb`

2. check Ipopt return value and msg --- currently there is a mismatch between the return value and return msg. It might caused by the version of Ipopt, and I just commented in a relevant issue in cyipopt.

3. terminate hiop if Ipopt failes to find one solution from all the initial points.

I do not see the same mismatch you are seeing. I the result of info.get('status', -1) is 0 and the status_msg indicates successful convergence of the interior-point solver.

@thartland
Copy link
Collaborator

thartland commented Apr 10, 2025

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.

@nychiang
Copy link
Collaborator Author

I do not see the same mismatch you are seeing. I the result of info.get('status', -1) is 0 and the status_msg indicates successful convergence of the interior-point solver.

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.

@thartland
Copy link
Collaborator

This looks great! I have some preliminary work started but once this is merged I'll start a new merge request to update hiopbbpy according to issues #720 and #721.

@cnpetra cnpetra merged commit 02b105e into develop Apr 10, 2025
7 checks passed
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.

3 participants