Skip to content

Commit

Permalink
Address comments from @jsiirola, @blnicho
Browse files Browse the repository at this point in the history
  • Loading branch information
mrmundt committed Feb 21, 2024
1 parent 40ad333 commit 1bccb16
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 142 deletions.
1 change: 0 additions & 1 deletion pyomo/common/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
# ___________________________________________________________________________

import argparse
import datetime
import enum
import os
import os.path
Expand Down
1 change: 1 addition & 0 deletions pyomo/contrib/appsi/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ def __init__(

class Solver(abc.ABC):
class Availability(enum.IntEnum):
"""Docstring"""
NotFound = 0
BadVersion = -1
BadLicense = -2
Expand Down
12 changes: 5 additions & 7 deletions pyomo/contrib/solver/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def solve(self, model: _BlockData, **kwargs) -> Results:
"""

@abc.abstractmethod
def available(self):
def available(self) -> bool:
"""Test if the solver is available on this system.
Nominally, this will return True if the solver interface is
Expand Down Expand Up @@ -159,7 +159,7 @@ def version(self) -> Tuple:
A tuple representing the version
"""

def is_persistent(self):
def is_persistent(self) -> bool:
"""
Returns
-------
Expand All @@ -178,9 +178,7 @@ class PersistentSolverBase(SolverBase):
Example usage can be seen in the Gurobi interface.
"""

CONFIG = PersistentSolverConfig()

@document_kwargs_from_configdict(CONFIG)
@document_kwargs_from_configdict(PersistentSolverConfig())
@abc.abstractmethod
def solve(self, model: _BlockData, **kwargs) -> Results:
super().solve(model, kwargs)
Expand Down Expand Up @@ -312,7 +310,7 @@ def remove_variables(self, variables: List[_GeneralVarData]):
"""

@abc.abstractmethod
def remove_params(self, params: List[_ParamData]):
def remove_parameters(self, params: List[_ParamData]):
"""
Remove parameters from the model
"""
Expand All @@ -336,7 +334,7 @@ def update_variables(self, variables: List[_GeneralVarData]):
"""

@abc.abstractmethod
def update_params(self):
def update_parameters(self):
"""
Update parameters on the model
"""
Expand Down
22 changes: 12 additions & 10 deletions pyomo/contrib/solver/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
NonNegativeInt,
ADVANCED_OPTION,
Bool,
Path,
)
from pyomo.common.log import LogStream
from pyomo.common.numeric_types import native_logical_types
Expand Down Expand Up @@ -74,17 +75,17 @@ def __init__(
ConfigValue(
domain=TextIO_or_Logger,
default=False,
description="""`tee` accepts :py:class:`bool`,
description="""``tee`` accepts :py:class:`bool`,
:py:class:`io.TextIOBase`, or :py:class:`logging.Logger`
(or a list of these types). ``True`` is mapped to
``sys.stdout``. The solver log will be printed to each of
these streams / destinations. """,
these streams / destinations.""",
),
)
self.working_dir: Optional[str] = self.declare(
self.working_dir: Optional[Path] = self.declare(
'working_dir',
ConfigValue(
domain=str,
domain=Path(),
default=None,
description="The directory in which generated files should be saved. "
"This replaces the `keepfiles` option.",
Expand Down Expand Up @@ -134,7 +135,8 @@ def __init__(
self.time_limit: Optional[float] = self.declare(
'time_limit',
ConfigValue(
domain=NonNegativeFloat, description="Time limit applied to the solver."
domain=NonNegativeFloat,
description="Time limit applied to the solver (in seconds).",
),
)
self.solver_options: ConfigDict = self.declare(
Expand Down Expand Up @@ -201,7 +203,7 @@ class AutoUpdateConfig(ConfigDict):
check_for_new_objective: bool
update_constraints: bool
update_vars: bool
update_params: bool
update_parameters: bool
update_named_expressions: bool
update_objective: bool
treat_fixed_vars_as_params: bool
Expand Down Expand Up @@ -257,7 +259,7 @@ def __init__(
description="""
If False, new/old parameters will not be automatically detected on subsequent
solves. Use False only when manually updating the solver with opt.add_parameters() and
opt.remove_params() or when you are certain parameters are not being added to /
opt.remove_parameters() or when you are certain parameters are not being added to /
removed from the model.""",
),
)
Expand Down Expand Up @@ -297,15 +299,15 @@ def __init__(
opt.update_variables() or when you are certain variables are not being modified.""",
),
)
self.update_params: bool = self.declare(
'update_params',
self.update_parameters: bool = self.declare(
'update_parameters',
ConfigValue(
domain=bool,
default=True,
description="""
If False, changes to parameter values will not be automatically detected on
subsequent solves. Use False only when manually updating the solver with
opt.update_params() or when you are certain parameters are not being modified.""",
opt.update_parameters() or when you are certain parameters are not being modified.""",
),
)
self.update_named_expressions: bool = self.declare(
Expand Down
4 changes: 2 additions & 2 deletions pyomo/contrib/solver/gurobi.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ def _remove_variables(self, variables: List[_GeneralVarData]):
self._mutable_bounds.pop(v_id, None)
self._needs_updated = True

def _remove_params(self, params: List[_ParamData]):
def _remove_parameters(self, params: List[_ParamData]):
pass

def _update_variables(self, variables: List[_GeneralVarData]):
Expand All @@ -770,7 +770,7 @@ def _update_variables(self, variables: List[_GeneralVarData]):
gurobipy_var.setAttr('vtype', vtype)
self._needs_updated = True

def update_params(self):
def update_parameters(self):
for con, helpers in self._mutable_helpers.items():
for helper in helpers:
helper.update()
Expand Down
79 changes: 15 additions & 64 deletions pyomo/contrib/solver/ipopt.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,10 @@
import subprocess
import datetime
import io
import sys
from typing import Mapping, Optional, Sequence

from pyomo.common import Executable
from pyomo.common.config import (
ConfigValue,
NonNegativeFloat,
document_kwargs_from_configdict,
ConfigDict,
)
from pyomo.common.config import ConfigValue, document_kwargs_from_configdict, ConfigDict
from pyomo.common.errors import PyomoException, DeveloperError
from pyomo.common.tempfiles import TempfileManager
from pyomo.common.timing import HierarchicalTimer
Expand Down Expand Up @@ -78,54 +72,7 @@ def __init__(
),
)
self.writer_config: ConfigDict = self.declare(
'writer_config', NLWriter.CONFIG()
)


class IpoptResults(Results):
def __init__(
self,
description=None,
doc=None,
implicit=False,
implicit_domain=None,
visibility=0,
):
super().__init__(
description=description,
doc=doc,
implicit=implicit,
implicit_domain=implicit_domain,
visibility=visibility,
)
self.timing_info.ipopt_excluding_nlp_functions: Optional[float] = (
self.timing_info.declare(
'ipopt_excluding_nlp_functions',
ConfigValue(
domain=NonNegativeFloat,
default=None,
description="Total CPU seconds in IPOPT without function evaluations.",
),
)
)
self.timing_info.nlp_function_evaluations: Optional[float] = (
self.timing_info.declare(
'nlp_function_evaluations',
ConfigValue(
domain=NonNegativeFloat,
default=None,
description="Total CPU seconds in NLP function evaluations.",
),
)
)
self.timing_info.total_seconds: Optional[float] = self.timing_info.declare(
'total_seconds',
ConfigValue(
domain=NonNegativeFloat,
default=None,
description="Total seconds in IPOPT. NOTE: Newer versions of IPOPT (3.14+) "
"no longer separate timing information.",
),
'writer_config', ConfigValue(default=NLWriter.CONFIG(), description="Configuration that controls options in the NL writer.")
)


Expand Down Expand Up @@ -416,11 +363,11 @@ def solve(self, model, **kwds):

if len(nl_info.variables) == 0:
if len(nl_info.eliminated_vars) == 0:
results = IpoptResults()
results = Results()
results.termination_condition = TerminationCondition.emptyModel
results.solution_loader = SolSolutionLoader(None, None)
else:
results = IpoptResults()
results = Results()
results.termination_condition = (
TerminationCondition.convergenceCriteriaSatisfied
)
Expand All @@ -435,18 +382,22 @@ def solve(self, model, **kwds):
results = self._parse_solution(sol_file, nl_info)
timer.stop('parse_sol')
else:
results = IpoptResults()
results = Results()
if process.returncode != 0:
results.extra_info.return_code = process.returncode
results.termination_condition = TerminationCondition.error
results.solution_loader = SolSolutionLoader(None, None)
else:
results.iteration_count = iters
results.timing_info.ipopt_excluding_nlp_functions = (
ipopt_time_nofunc
)
results.timing_info.nlp_function_evaluations = ipopt_time_func
results.timing_info.total_seconds = ipopt_total_time
if ipopt_time_nofunc is not None:
results.timing_info.ipopt_excluding_nlp_functions = (
ipopt_time_nofunc
)

if ipopt_time_func is not None:
results.timing_info.nlp_function_evaluations = ipopt_time_func
if ipopt_total_time is not None:
results.timing_info.total_seconds = ipopt_total_time
if (
config.raise_exception_on_nonoptimal_result
and results.solution_status != SolutionStatus.optimal
Expand Down Expand Up @@ -554,7 +505,7 @@ def _parse_ipopt_output(self, stream: io.StringIO):
return iters, nofunc_time, func_time, total_time

def _parse_solution(self, instream: io.TextIOBase, nl_info: NLWriterInfo):
results = IpoptResults()
results = Results()
res, sol_data = parse_sol_file(
sol_file=instream, nl_info=nl_info, result=results
)
Expand Down
16 changes: 8 additions & 8 deletions pyomo/contrib/solver/persistent.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,11 @@ def remove_variables(self, variables: List[_GeneralVarData]):
del self._vars[v_id]

@abc.abstractmethod
def _remove_params(self, params: List[_ParamData]):
def _remove_parameters(self, params: List[_ParamData]):
pass

def remove_params(self, params: List[_ParamData]):
self._remove_params(params)
def remove_parameters(self, params: List[_ParamData]):
self._remove_parameters(params)
for p in params:
del self._params[id(p)]

Expand All @@ -297,7 +297,7 @@ def remove_block(self, block):
)
)
)
self.remove_params(
self.remove_parameters(
list(
dict(
(id(p), p)
Expand Down Expand Up @@ -325,7 +325,7 @@ def update_variables(self, variables: List[_GeneralVarData]):
self._update_variables(variables)

@abc.abstractmethod
def update_params(self):
def update_parameters(self):
pass

def update(self, timer: HierarchicalTimer = None):
Expand Down Expand Up @@ -396,12 +396,12 @@ def update(self, timer: HierarchicalTimer = None):
self.remove_sos_constraints(old_sos)
timer.stop('cons')
timer.start('params')
self.remove_params(old_params)
self.remove_parameters(old_params)

# sticking this between removal and addition
# is important so that we don't do unnecessary work
if config.update_params:
self.update_params()
if config.update_parameters:
self.update_parameters()

self.add_parameters(new_params)
timer.stop('params')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ def setUp(self):
opt.config.auto_updates.check_for_new_or_removed_params = False
opt.config.auto_updates.check_for_new_or_removed_vars = False
opt.config.auto_updates.check_for_new_or_removed_constraints = False
opt.config.auto_updates.update_params = False
opt.config.auto_updates.update_parameters = False
opt.config.auto_updates.update_vars = False
opt.config.auto_updates.update_constraints = False
opt.config.auto_updates.update_named_expressions = False
Expand Down
Loading

0 comments on commit 1bccb16

Please sign in to comment.