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

change default behavior in appsi to not care where variables live #2903

Merged
merged 9 commits into from
Aug 8, 2023
11 changes: 7 additions & 4 deletions pyomo/contrib/appsi/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ def invalidate(self):


class PersistentBase(abc.ABC):
def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
self._model = None
self._active_constraints = dict() # maps constraint to (lower, body, upper)
self._vars = dict() # maps var id to (var, lb, ub, fixed, domain, value)
Expand Down Expand Up @@ -1334,9 +1334,6 @@ def update(self, timer: HierarchicalTimer = None):
self.remove_constraints(old_cons)
self.remove_sos_constraints(old_sos)
timer.stop('cons')
timer.start('vars')
self.remove_variables(old_vars)
timer.stop('vars')
timer.start('params')
self.remove_params(old_params)

Expand Down Expand Up @@ -1463,6 +1460,12 @@ def update(self, timer: HierarchicalTimer = None):
self.set_objective(pyomo_obj)
timer.stop('objective')

# this has to be done after the objective in case the
# old objective uses old variables
timer.start('vars')
self.remove_variables(old_vars)
timer.stop('vars')
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure I'm understanding, it has to be after constraints for the same reason, right? (It just already was.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, good point. Yes.

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'll update the comment



legacy_termination_condition_map = {
TerminationCondition.unknown: LegacyTerminationCondition.unknown,
Expand Down
2 changes: 1 addition & 1 deletion pyomo/contrib/appsi/solvers/cbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def __init__(


class Cbc(PersistentSolver):
def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
self._config = CbcConfig()
self._solver_options = dict()
self._writer = LPWriter(only_child_vars=only_child_vars)
Expand Down
2 changes: 1 addition & 1 deletion pyomo/contrib/appsi/solvers/cplex.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def __init__(self, solver):
class Cplex(PersistentSolver):
_available = None

def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
self._config = CplexConfig()
self._solver_options = dict()
self._writer = LPWriter(only_child_vars=only_child_vars)
Expand Down
2 changes: 1 addition & 1 deletion pyomo/contrib/appsi/solvers/gurobi.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class Gurobi(PersistentBase, PersistentSolver):
_available = None
_num_instances = 0

def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
super(Gurobi, self).__init__(only_child_vars=only_child_vars)
self._num_instances += 1
self._config = GurobiConfig()
Expand Down
2 changes: 1 addition & 1 deletion pyomo/contrib/appsi/solvers/highs.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class Highs(PersistentBase, PersistentSolver):

_available = None

def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
super().__init__(only_child_vars=only_child_vars)
self._config = HighsConfig()
self._solver_options = dict()
Expand Down
2 changes: 1 addition & 1 deletion pyomo/contrib/appsi/solvers/ipopt.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def __init__(


class Ipopt(PersistentSolver):
def __init__(self, only_child_vars=True):
def __init__(self, only_child_vars=False):
self._config = IpoptConfig()
self._solver_options = dict()
self._writer = NLWriter(only_child_vars=only_child_vars)
Expand Down
4 changes: 4 additions & 0 deletions pyomo/contrib/appsi/solvers/tests/test_gurobi_persistent.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ def test_quadratic_constraint_attr(self):
def test_var_attr(self):
m = pe.ConcreteModel()
m.x = pe.Var(within=pe.Binary)
m.obj = pe.Objective(expr=m.x)

opt = Gurobi()
opt.set_instance(m)
Expand Down Expand Up @@ -694,6 +695,8 @@ def test_update7(self):
m.y = pe.Var()

opt = self.opt
orig_only_child_vars = opt._only_child_vars
opt._only_child_vars = True
opt.set_instance(m)
self.assertEqual(opt._solver_model.getAttr('NumVars'), 2)

Expand All @@ -712,3 +715,4 @@ def test_update7(self):
opt.remove_variables([m.x])
opt.update()
self.assertEqual(opt._solver_model.getAttr('NumVars'), 1)
opt._only_child_vars = orig_only_child_vars
Loading