Bound calculations #224
Bound calculations #224
Conversation
…throws an error if the decomposition is a single OR
… constraints, and variables
get_or_make_var simplified
|
branches from not_operator |
# Conflicts: # cpmpy/expressions/globalconstraints.py # tests/test_globalconstraints.py
fixed bounds for Max
|
Rebased on master with countglobals. |
cpmpy/expressions/utils.py
Outdated
| @@ -109,3 +109,12 @@ def eval_comparison(str_op, lhs, rhs): | |||
| raise Exception("Not a known comparison:", str_op) | |||
|
|
|||
|
|
|||
| def get_bounds(expr): | |||
| # can return floats, use floor and ceil when creating an intvar! | |||
| if hasattr(expr,'get_bounds'): | |||
There was a problem hiding this comment.
I am not convinced by this.
I think it would be clearer to do
if isinstance(expr): return expr.get_bounds(); else return expr,expr?
There was a problem hiding this comment.
This would lead to circular import for Expression, I put the import in the get_bounds function
cpmpy/solvers/z3.py
Outdated
| @@ -353,6 +353,9 @@ def _z3_expr(self, cpm_con, reify=False): | |||
| elif rhs_is_expr and rhs.name == "element": | |||
| arr, idx = rhs.args | |||
| return self._z3_expr(all([(idx == i).implies(Comparison(cpm_con.name, lhs, arr[i])) for i in range(len(arr))])) | |||
| if isinstance(lhs,GlobalConstraint) and lhs.name == "count": | |||
| # I think this case is covered in the next line | |||
There was a problem hiding this comment.
I think this case is covered in th enext line, that this 'addition' is from a merge?
There was a problem hiding this comment.
true, I also removed the element case
There was a problem hiding this comment.
(we still have to change the element decomposition to allow indexes out of range, think ignace has that in another pr)
get_bounds uses isinstance
| else: | ||
| assert is_num(expr), f"All Expressions should have a get_bounds function, `{expr}`" | ||
| if is_bool(expr): | ||
| return 0, 1 |
There was a problem hiding this comment.
This is a known bool, right? Then we can return 0, 0 or 1, 1, depending on whether the bool is true or false.
There was a problem hiding this comment.
Pushed fixes. @IgnaceBleukx : can you do a quick sanity check on 5b3361d ? Then we can commit, and global decompose can grab the changes.
|
Looks good, maybe some documentation for the Did we do any profiling using bigtest? Maybe thats for a next iteration of this code? It will have a very high impact on the runtime of Good to merge for me! |
No description provided.