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

Appsi fbbt KeyError when Var bounded by Param #2574

Closed
emma58 opened this issue Oct 24, 2022 · 9 comments · Fixed by #3182
Closed

Appsi fbbt KeyError when Var bounded by Param #2574

emma58 opened this issue Oct 24, 2022 · 9 comments · Fixed by #3182

Comments

@emma58
Copy link
Contributor

emma58 commented Oct 24, 2022

Summary

There's a KeyError when using appsi fbbt on a model where there are Params in the Var bounds

Steps to reproduce the issue

from pyomo.environ import *
from pyomo.contrib import appsi

m = ConcreteModel()
m.p = Param(initialize=4)
m.x = Var(bounds=(0, m.p))
m.c = Constraint(expr=m.x >= 2)

m.pprint()

it = appsi.fbbt.IntervalTightener()
it.perform_fbbt(m)

m.pprint()

Error Message

Traceback (most recent call last):
  File "maybe.py", line 62, in <module>
    it.perform_fbbt(m)
  File "/home/esjohn/src/pyomo/pyomo/contrib/appsi/fbbt.py", line 219, in perform_fbbt
    self.set_instance(model, symbolic_solver_labels=symbolic_solver_labels)
  File "/home/esjohn/src/pyomo/pyomo/contrib/appsi/fbbt.py", line 94, in set_instance
    self.add_block(model)
  File "/home/esjohn/src/pyomo/pyomo/contrib/appsi/base.py", line 914, in add_block
    self.add_variables(list(dict((id(var), var) for var in block.component_data_objects(Var, descend_into=True)).values()))
  File "/home/esjohn/src/pyomo/pyomo/contrib/appsi/base.py", line 787, in add_variables
    self._add_variables(variables)
  File "/home/esjohn/src/pyomo/pyomo/contrib/appsi/fbbt.py", line 107, in _add_variables
    cmodel.process_pyomo_vars(self._pyomo_expr_types, variables, self._var_map, self._param_map,
KeyError: 140146219916720

Note that replacing m.p with value(m.p) works as expected.

Information on your system

Pyomo version: main
Python version: 3.8
Operating system: linux
How Pyomo was installed (PyPI, conda, source): source
Solver (if applicable):

@jsiirola
Copy link
Member

Note: as part of resolving this issue, we should revisit the use of APPSI FBBT in GDPopt (as disabled in #2575)

@bknueven
Copy link
Contributor

bknueven commented Mar 7, 2024

I just ran into this issue on current main. The same bug affects appsi_cbc and appsi_ipopt.

@bknueven
Copy link
Contributor

bknueven commented Mar 7, 2024

And the same thing affects constraint bounds:

from pyomo.environ import *                                                                                

m = ConcreteModel()
m.x = Var()
m.clb = Param(initialize=0)
m.c = Constraint(expr=(m.clb, m.x, None))

SolverFactory("appsi_ipopt").solve(m)

Result:

Traceback (most recent call last):
  File "/Users/bknueven/Software/pyomo_test.py", line 8, in <module>
    SolverFactory("appsi_ipopt").solve(m)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/base.py", line 1568, in solve
    results: Results = super(LegacySolverInterface, self).solve(model)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/solvers/ipopt.py", line 291, in solve
    self._writer.write(model, self._filename + '.nl', timer=timer)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/writers/nl_writer.py", line 240, in write
    self.set_instance(model)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/writers/nl_writer.py", line 76, in set_instance
    self.add_block(model)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/base.py", line 1114, in add_block
    self.add_constraints(
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/base.py", line 1032, in add_constraints
    self._add_constraints(cons)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/appsi/writers/nl_writer.py", line 115, in _add_constraints
    cmodel.process_nl_constraints(
KeyError: 140313948891280

@michaelbynum it seems like the constraint and variable bounds need to go through generate_standard_repn when converted to the cmodel to strip out non-mutable parameters, which don't need to be reflected in the cmodel anyways. I would be happy to implement that fix, but I'm wondering if there's a reason it isn't already done that way.

@mrmundt
Copy link
Contributor

mrmundt commented Mar 7, 2024

@bknueven - request: can you see if the same issue occurs using ipopt_v2? (This pulls the newer interface from pyomo.contrib.solver that we released as a preview)

@bknueven
Copy link
Contributor

bknueven commented Mar 7, 2024

@bknueven - request: can you see if the same issue occurs using ipopt_v2? (This pulls the newer interface from pyomo.contrib.solver that we released as a preview)

Good question -- ipopt_v2 does not have this bug, though I got an unrelated error when giving it the above model with no objective function.

Traceback (most recent call last):
  File "/Users/bknueven/Software/pyomo_test.py", line 9, in <module>
    SolverFactory("ipopt_v2").solve(m, tee=True)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/solver/base.py", line 518, in solve
    legacy_results, legacy_soln = self._map_results(model, results)
  File "/Users/bknueven/Software/pyomo/pyomo/contrib/solver/base.py", line 422, in _map_results
    if len(list(obj)) > 0:
TypeError: 'NoneType' object is not iterable

obj is None here with no objective.

@michaelbynum
Copy link
Contributor

The reason this is not accounted for is because I never use non-mutable parameters, so I never think about these edge cases.

I don't think we can run the bounds through generate_standard_repn because that will also remove the mutable parameters (note that mutable parameters in variable bounds work).

This might be fixed just by removing these two if statements:

if p.mutable:

if p.mutable:

@bknueven
Copy link
Contributor

bknueven commented Mar 7, 2024

Thanks Michael. That does seem to work, unsurprisingly.

What about the following patch? It keeps the non-mutable parameters out of the cmodel, at the cost of checking the mutable attribute.

diff --git a/pyomo/contrib/appsi/cmodel/src/expression.cpp b/pyomo/contrib/appsi/cmodel/src/expression.cpp
index 234ef47e8..8079de42b 100644
--- a/pyomo/contrib/appsi/cmodel/src/expression.cpp
+++ b/pyomo/contrib/appsi/cmodel/src/expression.cpp
@@ -1548,7 +1548,10 @@ appsi_operator_from_pyomo_expr(py::handle expr, py::handle var_map,
     break;
   }
   case param: {
-    res = param_map[expr_types.id(expr)].cast<std::shared_ptr<Node>>();
+    if (expr.attr("parent_component")().attr("mutable").cast<bool>())
+        res = param_map[expr_types.id(expr)].cast<std::shared_ptr<Node>>();
+    else
+        res = std::make_shared<Constant>(expr.attr("value").cast<double>());
     break;
   }
   case product: {

@michaelbynum
Copy link
Contributor

Looks great to me!

@mrmundt
Copy link
Contributor

mrmundt commented Mar 12, 2024

FYI, @bknueven , the bug you reported in ipopt_v2 has been corrected and is merged into main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants