Prevent crash when solver returns suffixes but not values for variables

[mfripp]

Fixes #597.

Summary/Motivation:

When the user requests an irreducibly infeasible set of constraints (IIS) for an infeasible mixed-integer program, cplexamp sometimes returns the IIS but not values for the variables. This currently causes pyomo to crash. Specifically, pyomo.opt.plugins.sol only creates entries in soln_variable for variables whose values have been returned by the solver, but then it tries to attach the IIS suffix value to a non-existent item in soln_variable, causing an uncaught KeyError().

There is already code in pyomo.opt.plugins.sol to prevent this problem with constraints. This pull request applies similar code to avoid this problem with variables.

The code below currently crashes Pyomo, but this error is fixed by the proposed changes:

from pyomo.environ import *
m = ConcreteModel()
m.x1 = Var(within=NonNegativeReals)
m.x2 = Var(within=NonNegativeIntegers)
m.constraint = Constraint(rule=lambda m: m.x1 + m.x2 <= -1)
m.objective = Objective(rule=lambda m: m.x1 + m.x2, sense=minimize)
m.iis = Suffix(direction=Suffix.IMPORT)
solver = SolverFactory('cplexamp')
res = solver.solve(m, options_string='iisfind=1', tee=True)
print("IIS:")
print("\n".join(sorted(c.name for c in m.iis)))

Changes proposed in this PR:

• add a dummy entry ({'Value': None}) to soln_variable if needed before applying the suffix. Note that this entry must include a 'Value' key in order to prevent other errors later.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[codecov-io]

Codecov Report

Merging #596 into master will increase coverage by 1.2%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master     #596     +/-   ##
=========================================
+ Coverage   66.02%   67.23%   +1.2%     
=========================================
  Files         374      393     +19     
  Lines       60314    62233   +1919     
=========================================
+ Hits        39825    41845   +2020     
+ Misses      20489    20388    -101

Impacted Files	Coverage Δ
pyomo/opt/plugins/sol.py	81% <100%> (+1.34%)	⬆️
pyomo/common/plugin.py	50% <0%> (-50%)	⬇️
pyomo/solvers/plugins/converter/pico.py	31.11% <0%> (-10.89%)	⬇️
pyomo/solvers/plugins/converter/ampl.py	34.78% <0%> (-4%)	⬇️
pyomo/contrib/example/plugins/ex_plugin.py	71.42% <0%> (-3.58%)	⬇️
pyomo/opt/plugins/res.py	52.38% <0%> (-2.17%)	⬇️
pyomo/contrib/gdpopt/nlp_solve.py	73.19% <0%> (-2.13%)	⬇️
pyomo/neos/plugins/NEOS.py	32.25% <0%> (-2.12%)	⬇️
pyomo/opt/plugins/dakota_text_io.py	73.84% <0%> (-1.52%)	⬇️
pyomo/solvers/plugins/converter/model.py	68.75% <0%> (-1.25%)	⬇️
... and 220 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0abdfd7...7aed807. Read the comment docs.

[qtothec]

@mfripp Thanks for the nice contribution. I remember running across problems figuring out how to use IIS when I was first getting started. I hope that someone on the team familiar with the ASL interface is able to review this soon.

[jsiirola]

The fix looks good to me. @mfripp: can you add a (small) test that exercises the new if check? We will probably want to run the test with the other CPLEX interfaces (LP, direct, persistent) as well.

[mfripp]

@jsiirola: Sure, happy to add a test. I took a quick look at the testing framework before I submitted this and another look just now, but couldn't figure out where to add a new test. Any advice?

[ghackebeil]

@mfripp: It might be easier to just upload an example .sol file that exercises this code. I can quickly put together a test using that.

[carldlaird]

@ghackebeil , were you able to get a test working? Would you like me to look at it?

[ghackebeil]

I have a test put together. I think the easiest thing to do, since this is on a fork, is to merge and I’ll add the test after. However, there is one controversial piece of this PR that may or may not need to change. Variable values are being set to None in the solution rather than just leaving them alone (i.e., by just initializing the solution entry to an empty dict).

[qtothec]

@ghackebeil You should be able to push directly to the fork. Most people leave "allow edits from maintainers" checked.

[ghackebeil]

@qtothec: Good point

[ghackebeil]

Turns out I do not have push access. @mfripp: Can you change these settings, at least temporarily?

[mfripp]

Sorry, I didn't see the July 24 response until just now. Let me know if I can help with anything.

[ghackebeil]

@mfripp: No problem. I have some local commits made to your fork that I'd like to push before we merge this PR. Would you mind enabling push access for me (at least temporarily) on your pyomo fork. I assume that can be done somewhere in the repository settings.

[mfripp]

OK, I added you as a collaborator on https://github.com/mfripp/pyomo. I think that should give you push access. Let me know if I can help with anything else. Matthias

[ghackebeil]

I believe this is ready to merge, pending someone else's approval.