Solution Pool #3657
Solution Pool #3657
Conversation
Reordering and documenting API
Use the Pyomo Bunch class as an alias for Munch, to avoid introducing an additional Pyomo dependency.
|
Note that this PR now includes updates to the Bunch class defined in pyomo.common. I have been using the public Munch class, but these revisions align the Bunch API with Munch. |
Avoiding use of KW_ONLY, which is an internal mechanism
Using new serialization API, which is simpler. :)
1. Reworking solver matrix logic 2. Fixing test to benchmark against the solution values
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3657 +/- ##
==========================================
- Coverage 89.31% 87.20% -2.12%
==========================================
Files 896 826 -70
Lines 103682 102378 -1304
==========================================
- Hits 92605 89280 -3325
- Misses 11077 13098 +2021
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Added non-positive error check and value 1 warning for num_solutions in balas
Added num_solution error if num_solutions is non-positive, warning if num_solutions =1
emma58
left a comment
emma58
left a comment
There was a problem hiding this comment.
Sorry for the huuuuge number of comments, but there's a lot here! Many are minor, but there are a few design questions too: The biggest one is that I'm wondering if the VariableInfo and ObjectiveInfo classes could store a reference to the original pyomo component rather than copying a lot but not all of the same data. It seems like that would make it much easier to map solutions in the pool to the original model, which I imagine will be a common user need.
pyomo/contrib/alternative_solutions/tests/test_gurobi_lp_enum.py
Outdated
Show resolved
Hide resolved
|
@emma58 Thanks for all the feedback! I've gone through some of your comments, but I'll get back to this sometime next week. |
| @@ -0,0 +1,130 @@ | |||
| # ___________________________________________________________________________ | |||
There was a problem hiding this comment.
This module name is misleading: I expected this to define a derived SolutionPool class that was specific to the gurobipy interface.
There was a problem hiding this comment.
What would you suggest? Something like gurobi_with_native_solution_pool.py seems verbose.
| objective : None or :py:class:`ObjectiveInfo` | ||
| A :py:class:`ObjectiveInfo` object. (default is None) | ||
| objectives : None or list | ||
| A list of :py:class:`ObjectiveInfo` objects. (default is None) |
There was a problem hiding this comment.
I find the duplicative API confusing. It would be simpler and less error-prone for the __init__ to take a single argument, and then in the constructor map a single ObjectiveData to a list of length 1 (you can also then accept an IndexedObjective and map it to a list of Objectives!)
(To the point about "error prone": note that the current implementation for processing the "2 argument solution" has logic errors and will silently drop objectives if both objective= and objectives= are specified.)
| @dataclasses.dataclass(**dataclass_kwargs) | ||
| class VariableInfo: | ||
| """ | ||
| A class to store solutions from a Pyomo model. | ||
| Represents a variable in a solution. | ||
| Attributes | ||
| ---------- | ||
| variables : ComponentMap | ||
| A map between Pyomo variables and their values for a solution. | ||
| fixed_vars : ComponentSet | ||
| The set of Pyomo variables that are fixed in a solution. | ||
| objective : ComponentMap | ||
| A map between Pyomo objectives and their values for a solution. | ||
| Methods | ||
| ------- | ||
| pprint(): | ||
| Prints a solution. | ||
| get_variable_name_values(self, ignore_fixed_vars=False): | ||
| Get a dictionary of variable name-variable value pairs. | ||
| get_fixed_variable_names(self): | ||
| Get a list of fixed-variable names. | ||
| get_objective_name_values(self): | ||
| Get a dictionary of objective name-objective value pairs. | ||
| value : float | ||
| The value of the variable. | ||
| fixed : bool | ||
| If True, then the variable was fixed during optimization. | ||
| name : str | ||
| The name of the variable. | ||
| index : int | ||
| The unique identifier for this variable. | ||
| discrete : bool | ||
| If True, then this is a discrete variable | ||
| suffix : dict |
There was a problem hiding this comment.
Are there any applications using a Solution (Pool) from Pyomo that are making use of the feature that the pool is disconnected from Pyomo? This seems like a design decision that is adding a lot of complexity and potential for logic errors for a use case that doesn't exist?
For example, you are re-inventing solutions for caching Pyomo objects that have already been well-defined and worked out / debugged elsewhere (e.g., pyomo.common.collections; pyomo.contrib.solver).
Further, the dev team has been spending a significant amount of effort to remove the use of string names from other parts of Pyomo (e.g., parmest), so going and adding a new API that is string-based seems to run counter to that effort.
1. Renaming kwds to kwargs to clarify the intent to capture all keyword arguments. 2. Refactoring PyomoSolution into a class that inherits from Solution.
This supported compatibility with Munch, but this isn't being used.
|
It's not obvious to me why the linux slim tests are failing. Do we need to conditionally import numpy? |
6 participants
Fixes N/A (partially addresses #3513) .
Summary/Motivation:
The alternative_solutions contrib package includes a rudimentary solution pool object. This PR includes a more comprehensive capability for defining and managing solution pools.
Changes proposed in this PR:
NOTE: This is a WIP PR. I'm submitting this now to help define expectations for finalizing this new capability.
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: