Generalize ReduceEqualityMaskRange - #742
Conversation
3d226e0 to
2e446b5
Compare
0e4c9a7 to
b4593f8
Compare
2f1488b to
7a3a018
Compare
| for solution in _solve_stream_equality(cond, streams) | ||
| if cls._can_restrict(streams[solution.stream_op]) | ||
| ] | ||
| ranked_conds = sorted(matched_conds, key=lambda c: (*cls._rhs_cost(c[2]), c[0])) |
There was a problem hiding this comment.
What is being added by this new cost-based sorting step here? If it's important, is there a way to achieve the same thing through choose_contraction, so there's only a single source of cost-based non-determinism? If it's not important, can we just retain the original order in matched_conds and remove it?
| return {apply: _apply} | ||
|
|
||
|
|
||
| def sizeof(term: Expr) -> int: |
There was a problem hiding this comment.
I only see one place in ops.monoid that uses sizeof. If this can't be removed, do you expect it to be used anywhere else in the future? It seems like a very questionable basis for a cost model.
There was a problem hiding this comment.
This code is still in flux, so I'm not sure. In the robotl use case, effectful overhead is the main cost (both before and after the caching work). A cost model that reduces term growth is essential. I'd expect future rewriting rules to have similar problems with creating huge terms, so I expect sizeof to remain useful.
2 participants
No description provided.