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

Solver Refactor Part 1: Introducing the new solver interface #3137

Merged
merged 281 commits into from
Feb 21, 2024

Conversation

mrmundt
Copy link
Contributor

@mrmundt mrmundt commented Feb 14, 2024

Fixes #1030 (but not entirely)

Summary/Motivation:

This is a relatively large PR that introduces a refactored solver interface (placed in contrib as it is not yet the default behavior). Ideally, the intent behind this PR is to:

  1. Standardize behavior for solvers
  2. Make behavior more intuitive
  3. Incorporate necessary features such as timers, logging, etc.
  4. Create a better foundation for new interfaces

Changes proposed in this PR:

  • New base solver interface for direct and persistent solvers, based on APPSI
  • Changed structure for options and results
  • Framework to remove unclear/obsolete/inefficient options (such as keepfiles)
  • Documentation for solver interfaces (new and exciting)
  • Backwards compatibility mapping to allow users to use both the new and old interfaces at the same time
  • Minor bug fixes that were caught while refactoring

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.

Copy link
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still going through this but here are some initial comments

doc/OnlineDocs/developer_reference/solvers.rst Outdated Show resolved Hide resolved
doc/OnlineDocs/developer_reference/solvers.rst Outdated Show resolved Hide resolved
pyomo/contrib/solver/base.py Outdated Show resolved Hide resolved
pyomo/__future__.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/sol_reader.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/sol_reader.py Outdated Show resolved Hide resolved
@mrmundt mrmundt requested a review from blnicho February 20, 2024 17:31
pyomo/contrib/solver/config.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/config.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/ipopt.py Show resolved Hide resolved
Comment on lines +32 to +34
def parse_sol_file(
sol_file: io.TextIOBase, nl_info: NLWriterInfo, result: Results
) -> Tuple[Results, SolFileData]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should re-think this API and not pass the results in / out of this function. I would argue for this to be a parser that returns the parsed data, and populating a results object is the responsibility of the caller. This would also remove the somewhat confusing passing in / out of results objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. It's "rough" but not "as rough as it used to be" right meow.

pyomo/contrib/solver/tests/unit/test_ipopt.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/tests/unit/test_results.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/base.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/base.py Outdated Show resolved Hide resolved
pyomo/contrib/solver/tests/solvers/test_solvers.py Outdated Show resolved Hide resolved
@michaelbynum michaelbynum merged commit 70b2803 into Pyomo:main Feb 21, 2024
33 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.

[PEP] Redesign of Pyomo Solvers
6 participants