feat(atenlib): add op(index put) #522
Conversation
Codecov Report
@@ Coverage Diff @@
## main #522 +/- ##
==========================================
+ Coverage 73.75% 73.82% +0.07%
==========================================
Files 109 109
Lines 10882 10917 +35
Branches 1131 1135 +4
==========================================
+ Hits 8026 8060 +34
+ Misses 2551 2550 -1
- Partials 305 307 +2
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
LMK if you are able to fix the test errors? I can take a look tomorrow too |
Yes, please. It is in graph_building.py. |
| """index_put(Tensor self, Tensor?[] indices, Tensor values, bool accumulate=False) -> Tensor""" | ||
|
|
||
| raise NotImplementedError() | ||
| index = op.SequenceAt(indices, 0) # assume indices only have 1 element |
There was a problem hiding this comment.
I am bit unclear about the assumption above. Is this just for now? Is the input type expected to be changed to INT64 ?
There was a problem hiding this comment.
Looks like indices is a list of 1d tensors, so INT64 may not do it?
There was a problem hiding this comment.
On this line, was there a reason we assume indices only has one element?
There was a problem hiding this comment.
From the test case, all the indices have only one element. If it contains multiple elements, we need to use for/while to do the logic.
But for this aten op, the multiple elements can be combined to one element, for example, indices=[[0,1], [2,3]], can be combined to indices=[[0,1,2,3],].
There was a problem hiding this comment.
Should we do a sequenceConcat or something like that?
There was a problem hiding this comment.
I tried in Torch, actually it cannot work like this:
torch.index_put( self=tensor(5x5), indices=(tensor([0]), tensor([1]),), values=tensor(2x5))
so we don't know how to use more than one element in indices tuple.
|
Graph builder fixed in #528 |
No description provided.