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

[WIP] CyIpopt callback for displaying variables/constraints causing infeasibilities #2860

Closed
wants to merge 35 commits into from

Conversation

Robbybp
Copy link
Contributor

@Robbybp Robbybp commented May 31, 2023

Summary/Motivation:

One advantage of using CyIpopt over Ipopt is the ability to interact with the solver via a callback. Another advantage is that we can associate the solver's data structures with Pyomo variables and constraints, which might have more meaning for a user. This PR combines these capabilities by introducing a callback that displays the variables and constraints that correspond to the highest-magnitude infeasibilities each iteration of a solve.

This PR will not work until CyIpopt publishes a new release, but I wanted to open it for feedback and possibly solicit some ambitious users.

This PR needs:

  • Unit tests (capture output and make sure it is as expected)
  • Practical testing debugging real problems
  • CyIpopt release

Ideally, eventually, we could automatically link CyIpopt to the IDAES-provided Ipopt library, so that users can obtain this functionality with a simple pip install cyipopt. Then I envision the callback being enabled with something like

solver = pyo.SolverFactory("cyipopt")
solver.options["infeasibility_callback"] = True

Breaking change

Note that this is a breaking change to the call signature of the intermediate callback that our CyIpopt interface, CyIpoptNLP, expects. We now expect an argument for self, the instance of cyipopt.Problem, which is necessary for calling the get_current_iterate and get_current_violations methods. We could think about how to do this in a non-breaking manner (using inspect to branch on the signature of the callback) or how to update the signature to be more extensible (accept a dictionary as an argument that we can add to if/when ever we want to add more arguments to the callback).

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@Robbybp
Copy link
Contributor Author

Robbybp commented Jun 7, 2023

@blnicho Raised the possibility of forking CyIpopt and creating a tag that we can pin against for testing. Another advantage of forking CyIpopt is that we could (potentially) patch it to search for the IDAES-installed Ipopt, so that cyipopt (and therefore this callback) would be available with simply:

$ idaes get-extensions
$ pip install git+https://github.com/our/fork.git@tag

Not as simple as pip install cyipopt but still pretty good.

@Robbybp
Copy link
Contributor Author

Robbybp commented Jul 7, 2023

Tests are failing as I have broken the callback API. This can be handled by using inspect.signature to branch on the call signature of the user-provided callback and only provide the new self (instance of cyipopt.Problem) argument if the callback accepts a sufficient number of arguments.

The alternative is to just break the callback API. This doesn't seem like a good idea, as we have callback examples in pyomo.contrib.pynumero.examples.callback. These should be updated to the new API as part of this PR, but we probably shouldn't just pull the rug out from under anybody using this feature (without a proper deprecation warning + release cycle).

@Robbybp
Copy link
Contributor Author

Robbybp commented Jul 24, 2023

Windows coverage failure appears unrelated

@blnicho
Copy link
Member

blnicho commented Sep 25, 2023

@Robbybp looks like cyipopt 1.3.0 was released a couple days ago so this PR can now move forward when you have time to come back to it.

@Robbybp
Copy link
Contributor Author

Robbybp commented Sep 25, 2023

@blnicho Thanks for the FYI. I want to test this out with some users other than myself before merging. The CyIpopt release will make this much easier.

@Robbybp
Copy link
Contributor Author

Robbybp commented Feb 19, 2024

I'm actually starting to use this, and I think this needs a bit more development before it is ready, so I'm going to close this PR for now. Things I need to think about:

  • As I start caching more data on the callback class, I need to make sure that instances CyIpoptCallback are not re-used across different models, and maybe across different Ipopt solves
  • I need to think about what data to cache, how to store it, and whether this should be handled by the base class.
  • What helper methods should be included to make use of the infeasibility data stored on the instance?

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.

2 participants