Open michaelbynum opened 5 years ago
I think you should reduce the scope of this PEP. The third bullet point is an orthogonal and controversial issue. There is no need to have it possibly get in the way the other (much needed) changes.
Regarding the third bullet Gabe references, can you clarify? Is said variable at least below the current block in the model component hierarchy? If it's not in the hierarchy, then I agree with Gabe that this is highly controversial.
I updated the motivation in the pep to have numbers.
It would be good to document the cases where "Bullet 3" is required. I also admit that the first time I heard that we needed to support solves where variables live outside the hierarchy, I thought this was very strange because it breaks encapsulation of sub-models. Having said that, I would like to see a use case where this is required to be convinced one way or another.
@ghackebeil @jwatsonnm I will open a separate PEP for (3).
@ghackebeil @jwatsonnm @carldlaird Let's move the discussion of (3) to #1032.
Now that the controversy has moved to #1032, I predict that everyone will think the new PEP with only items one and two is non-controversial great idea and that you should proceed with your plan to create a couple of examples.
I am adding a performance label to this PEP. I believe that a redesign of the base solver API is a prerequisite before we can (effectively) tackle things like reworking the writers / readers / persistent APIs.
Archived on the master Performance Proposals Issue (#1430). Not closing as this is also marked as a PEP.
It would be very useful to users to define one or more base protocols or base classes that all solvers inherit from. Many solvers like the BARON shell interface inherit from opt.base.solvers.OptSolver
. However, several solver managers including GAMS and AsynchronousActionManager
and MindtPySolver
do not. This causes problems like #2757, where users expect options=
to work and it doesn't.
It would be best to factor out some standard behavior, like supporting the options=
keyword to set .options
.
Also, for example, all solvers seem to implement a solve
method, and many have a license_is_valid
method. Defining such methods in an abstract base class or protocol would help users keep track of what solvers can even do.
@alexchandel - Agreed. All of those considerations are on our list.
I would add to this—because the "results" of a Pyomo solver are part of the solver's API—to standardize at least SOME of the SolverResults
structure across all solvers.
In particular, indicate in universal fashion whether the result was a solution, or infeasible/failure. See #2942 for an example where this behavior is inconsistent between solvers, preventing universal client code from being written.
I would add an additional motivation for base classes is the recent emergence of powerful Python type checkers like Pylance/Pyright and mypy. Unfortunately some of Pyomo is extremely dynamic, and currently difficult for type checkers to work out (e.g. #2078, #2821, #2938). However, it would be nice for the construction SolverFactory(backend.str())
to be known to return a subclass of a known base class with a .solve()
method, and similarly for SolverManagerFactory(backend.str())
.
Additionally, the Pyomo solver interface needs a common exception for when a Solver is not available. Observe the extremely different behavior when calling solve() on two solvers, neither of which is available:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/alex/test/test.py", line 3370, in start
main()
…
File "/Users/alex/test/.venv/lib/python3.11/site-packages/pyomo/solvers/plugins/solvers/GAMS.py", line 766, in solve
self.available()
File "/Users/alex/test/.venv/lib/python3.11/site-packages/pyomo/solvers/plugins/solvers/GAMS.py", line 654, in available
raise NameError(
NameError: No 'gams' command found on system PATH - GAMS shell solver functionality is not available.
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/alex/test/test.py", line 3370, in start
main()
…
File "/Users/alex/test/.venv/lib/python3.11/site-packages/pyomo/opt/base/solvers.py", line 533, in solve
self.available(exception_flag=True)
File "/Users/alex/test/.venv/lib/python3.11/site-packages/pyomo/opt/solver/shellcmd.py", line 141, in available
raise ApplicationError(msg % self.name)
pyomo.common.errors.ApplicationError: No executable found for solver 'baron'
Linking to issue #728. It would be nice if this utility for extracting info from the Ipopt log could be incorporated into the redesigned Ipopt interface.
Linking to issue #7
To do for Part 2:
pyomo.contrib.solver.sol_reader
to be an actual parser (e.g., not pass back and forth Results
objects)solver_options
and name
to two different values such as interface
and algorithm
(see https://github.com/Pyomo/pyomo/pull/3137#discussion_r1492898399, https://github.com/Pyomo/pyomo/pull/3137#discussion_r1492986691)MixIns
rather than inherit from a base class_VarAndNamedExprCollector
to inherit from StreamBasedExpressionVisitor
(see https://github.com/Pyomo/pyomo/pull/3137#discussion_r1492935431)Availability
its own class rather than a class-level enum (see https://github.com/Pyomo/pyomo/pull/3137#discussion_r1492988651)SubprocessXXX
system (see https://github.com/Pyomo/pyomo/pull/3137#discussion_r1492994771)TeeStream
(see https://github.com/Pyomo/pyomo/pull/3137#discussion_r1493004547)Ipopt.solve
into not such a horribly monstrous method (already underway in mrmundt:solver-refactor-pt2
)I think the second bullet should be two different things:
1) shift name
and type
to two different values such as interface
and algorithm
, and
2) Generalize (and perhaps rename?) solver_options
to handle case where multiple subsolvers are involved, all of which could have their own options.
I would like to start redesigning the solver interfaces in Pyomo.
Motivation
Preliminary Design Decisions (to be updated with time)
Implementation
Thoughts? Concerns?
Related Issues
7
19
22
51
175
296
312
371
380
387
548
728
768
798
861
868
915
1023
1032
1074
1088
1169
1275
1333
1401
1466
1487
1490
1567
1615
1776 🇺🇸
1811
1900
1926
2130
2167
2218
2362
2401
2452
2484
2485
2545
2564
2611
2703
2757
2785
2807
2845
2908
2955
2958
3000
3064
3107
3110
3147
3191