ConstraintSet should override list and return NotImplemented for addition and multiplication

[blakecole]

@bqpd @pgkirsch

The documentation on Tight ConstraintSets states the following:

"Tight ConstraintSets will warn if any inequalities they contain are not tight (that is, the right side does not equal the left side) after solving. This is useful when you know that a constraint should be tight for a given model, but representing it as an equality would be non-convex."

This seems to suggest that the use of tight constraints (via the Tight() function) will not alter or otherwise interrupt a GPKit solution; however, I have recently discovered that this is not always the case.

While attempting to solve a model, I noticed that GPKit would throw an error only when a particular constraint had been specified as Tight(). Without the use of the Tight() function, GPKit was able to find a solution to the model.

The nature of the error message was somewhat befuddling: it appeared that when the constraint was specified as Tight, the program was no longer able to interpret one or more constant variables. The error read:

Traceback (most recent call last):
File "tight_error.py", line 63, in
tsol = m.solve(verbosity=1)
File "/Users/bicole/GitHub/gpkit/gpkit/constraints/prog_factories.py", line 132, in solvefn
self.program, progsolve = genfunction(self, **kwargs)
File "/Users/bicole/GitHub/gpkit/gpkit/constraints/prog_factories.py", line 88, in programfn
prog = program(self.cost, self, constants, **initargs)
File "/Users/bicole/GitHub/gpkit/gpkit/constraints/gp.py", line 95, in init
self.check_bounds(err_on_missing_bounds=True)
File "/Users/bicole/GitHub/gpkit/gpkit/constraints/gp.py", line 121, in check_bounds
for (v, b), x in missingbounds.items()))
gpkit.exceptions.UnboundedGP: m_3 has no lower bound.

Again, when the Tight() function is removed from the constraint, the model is able to find a solution without any issue.

I have attached a minimum working example for your review. The boolean variable toggleTight can be set to True to incite the issue.

Thanks in advance,
-Blake
tight_error.py.zip

[whoburg]

great ticket - thanks for the report. This one could be good for new contributors. And the provided MWE looks like a great unit test

• add unit test based on MWE

[pgkirsch]

Hey @blakecole thanks for putting this together!

A couple observations:

1. It looks like it may not have anything to do with Tight specifically. Replacing Tight with ConstraintSet (the parent class of Tight) yields the same error.

-  constraints += Tight([m_1 * d_1 >= m_2 * d_2 + m_3 * d_3])
+  constraints += ConstraintSet([m_1 * d_1 >= m_2 * d_2 + m_3 * d_3])

2. One difference between the two options toggled by toggleTight is that the non-tight-wrapped constraint is in a list. Putting the Tight constraint set (or just the ConstraintSet) in a list actually also solves the error too, though I'm not sure why yet..

-  constraints += Tight([m_1 * d_1 >= m_2 * d_2 + m_3 * d_3])
+  constraints += [Tight([m_1 * d_1 >= m_2 * d_2 + m_3 * d_3])]

Regardless of the above, this definitely feels like a bug or at the very least confusing behaviour for the user.

I hope you don't mind, I pared down/tweaked the MWE a little:

from gpkit import Variable, Model
from gpkit.constraints.set import ConstraintSet

option = 1

m_1 = Variable("m_1",        "kg", "mass 1")
m_2 = Variable("m_2",        "kg", "mass 2")
m_3 = Variable("m_3", 0.6,   "kg", "mass 3")
m_a = Variable("m_a", 0.138, "kg", "mass a")

constraints = [m_2 >= m_a]
if option == 1: # does not work
    constraints += ConstraintSet([m_1 >= m_3])
elif option == 2: # works
    constraints += [ConstraintSet([m_1 >= m_3])]
elif option == 3: # works
    constraints += [m_1 >= m_3]

costfn = m_1 + m_2
m = Model(costfn, constraints)
tsol = m.solve()

Btw @whoburg and @bqpd, Blake is a former Hyperloop One colleague of mine, currently finishing a PhD at MIT doing some awesome work optimizing saildrone design using GP (he came across gpkit independently). The ecosystem of gpkit applications continues to grow!

[pgkirsch]

Another observation: option 1 only breaks if m_3 is not used elsewhere (in the main constraints list).

[blakecole]

@pgkirsch the pared down MWE looks great to me -- I prioritized speed over minimalism! Thanks for that near-term fix, good to know that putting the tight constraint in a list solves the problem.

So I guess maybe Tight is intended to bracket only the constraint expression itself, and then the entire thing goes in a list? I.e:
constraint += [Tight(inequality expression)]

I suppose that would sort of make sense, now that I think about it. That would enable one to build a single list of comma separated constraints, while specifying only some of them as Tight.

[whoburg]

Ahh great observation by @pgkirsch. Yeah now looking back at the original MWE I think that fully explains it. Propose closing this (as a Python gotcha) unless anyone can think of a specific improvement to make or test to add.

[bqpd]

Late to the party, but welcome @blakecole!

All ConstraintSets take as their first argument lists or dicts whose values are list-like things, and they're generally used recursively: ContraintSets of ConstraintSets etc.

...but since they inherit from lists, they unfortunately permit addition to them despite needing some data that gets destroyed in that addition (particularly fixed-variable values). Oops. They probably shouldn't let you do that; we can keep this issue open until they return NotImplemented on addition and multiplication.

helpfully, the given example would work if m_3 had been declared with

m_3    = Variable("m_3",    0.6,    "kg",  "mass 3", constant=True)

but @pgkirsch's solution is much preferred.

[blakecole]

Good to know -- thanks all, for the insights! Please let me know when / if you'd like me to close out this ticket.

[whoburg]

Big fan of @bqpd's suggestion of ConstraintSet returning NotImplemented on addition and multiplication.