Misc. fixes from concatenation - #512
Conversation
6852b86 to
9427bda
Compare
inducer
left a comment
There was a problem hiding this comment.
LGTM. Just one question, out of curiosity, mostly.
| (1,)*i_adv_indices[0]+adv_idx_shape)) | ||
|
|
||
| if not idx.tags_of_type(AssumeNonNegative): | ||
| if axis_len > 1 and not idx.tags_of_type(AssumeNonNegative): |
There was a problem hiding this comment.
Could you add some context here? (Maybe in a comment?) Under what circumstances was this relevant?
There was a problem hiding this comment.
I hope this need for simplification arose while we were indexing into a 1-long axes and not because we were allowing building advanced indexing expressions into a 0-long axis. :)
There was a problem hiding this comment.
Hmm. Well, at one point IIRC I was seeing index lambdas with a lot of unnecessary % 1s in their exprs. But now I'm no longer able to reproduce that. Maybe it was just being triggered by something else that was wrong in an intermediate version of the concatenation code, that has since been fixed? 🤷♂️ I've reverted the change for now, I can make a new PR if I run into it again.
| (1,)*i_adv_indices[0]+adv_idx_shape)) | ||
|
|
||
| if not idx.tags_of_type(AssumeNonNegative): | ||
| if axis_len > 1 and not idx.tags_of_type(AssumeNonNegative): |
There was a problem hiding this comment.
I hope this need for simplification arose while we were indexing into a 1-long axes and not because we were allowing building advanced indexing expressions into a 0-long axis. :)
9427bda to
384a742
Compare
384a742 to
e89a8d5
Compare
|
Thx! |
A few small fixes from the WIP concatenation branch that don't fit anywhere else.