Add asumption for unique indices#2225
Conversation
ricardoV94
added
enhancement
ricardoV94
force-pushed
the
assume_unique_indices
branch
from
84f08aa to
2956155
Compare
| def _has_unique_indices(fgraph, idx) -> bool: | ||
| """Whether ``idx``'s entries are provably duplicate-free: a constant with | ||
| unique entries, or a variable asserted ``unique_indices`` by the user.""" | ||
| return _constant_has_unique_indices(idx) or check_assumption( |
There was a problem hiding this comment.
Do you have a sense of which of these is cheaper? It's the assumption check if the Feature is already attached, but in live code i don't know.
There was a problem hiding this comment.
I don't think it matters, people aren't going to put asumptions on constant indices (no need anyway)? Either way after the first check it's cached
ricardoV94
force-pushed
the
assume_unique_indices
branch
from
2956155 to
e565cff
Compare
| Assert that *x* is (or is not) a selection matrix | ||
| permutation : bool, optional | ||
| Assert that *x* is (or is not) a permutation matrix | ||
| unique_indices : bool, optional |
There was a problem hiding this comment.
Should we reject this flag on non-1d inputs for now? Otherwise we need to know which axis/axes have unique indices, and we don't have the machinery for parameterized assumptions yet. If you want to add it though I'd be happy, I want it for matrix rank among other things.
There was a problem hiding this comment.
this assumes unique over the whole data, I don't care about dims
There was a problem hiding this comment.
(not yet), the guard against repeated computations still needs some work, but I don't think people are gonna do (symbolic) advanced matrix indexing and want it to lift, they could always do flat and reshape
2 participants
This allows symbolic indices to be lifted towards the inputs. There is no cleaner / structural way to do this in PyTensor