Conversation
|
test models please |
|
Looks like one of the engine tests doesn't run with cvxopt...anyway @whoburg this is ready for review and merging. |
|
|
||
| def _simplify_posy_ineq(self, exps, cs): | ||
| def _check_constraint(self, exps, cs): | ||
| "Simplify a posy <= 1 by moving constants to the right side." |
There was a problem hiding this comment.
Let's improve this docstring and possibly the method name. What question does the boolean return value answer? What condition is _check_constraint checking?
| # because they allow models to impose requirements | ||
| return (), np.array([]) | ||
| return False | ||
| return exps, cs |
There was a problem hiding this comment.
It's confusing that this method now sometimes returns a bool and sometimes returns exps, cs. Is that intended and does it really need to be that way?
| "be converted to units of %s" % | ||
| (self.p_lt, self.m_gt)) | ||
|
|
||
| p.exps, p.cs = self._simplify_posy_ineq(p.exps, p.cs) |
There was a problem hiding this comment.
Not clear to me why we should remove simplification. Seems like this was a mapping bug, not a simplification bug, right? Isn't this only an indirect fix?
|
Yup, I have a working branch which addresses this better. Will push soon! |
|
Superseded by #1064 |
2 participants
Closes #1060