Support negative indices in ndarray.__getitem__ #107688
Conversation
In this case, we copy, but this is part of the set of divergences described in Quansight-Labs/numpy_pytorch_interop#73. This does not work with dynamic shapes, but it's not clear to me what would be the best fix [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/107688
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d85d9a5 with merge base ba5eeed ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
lezcano
added
module: numpy
|
@ezyang what's the best way to support this for dynamic shapes? |
In this case, we copy, but this is part of the set of divergences described in Quansight-Labs/numpy_pytorch_interop#73. This does not work with dynamic shapes, but it's not clear to me what would be the best fix cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov [ghstack-poisoned]
In this case, we copy, but this is part of the set of divergences described in Quansight-Labs/numpy_pytorch_interop#73. This does not work with dynamic shapes, but it's not clear to me what would be the best fix cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov [ghstack-poisoned]
There was a problem hiding this comment.
The reason it doesn't work with dynamic shapes is because Python slice() is dumb and forces an __index__ call. You just need to rewrite the code to not use slice. This usually means calling some lower level PyTorch API like tensor.narrow; you can figure out what the getitem call desugared to by looking at the resulting ops in the graph.
|
I think the issue here is that I'm just doing an unguarded "if slice.step < 0" within the function. Now, I don't know whether we should start putting guards in the decomposition... that doesn't sound right. |
|
Oh, you have another problem which is that you can't actually call If you think my advice is not relevant, you should post the errors. |
In this case, we copy, but this is part of the set of divergences described in Quansight-Labs/numpy_pytorch_interop#73. This does not work with dynamic shapes, but it's not clear to me what would be the best fix cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov [ghstack-poisoned]
|
#107689 calls our custom |
In this case, we copy, but this is part of the set of divergences described in Quansight-Labs/numpy_pytorch_interop#73. This does not work with dynamic shapes, but it's not clear to me what would be the best fix cc mruberry rgommers voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov [ghstack-poisoned]
This issue was discovered once we were able to trace without breaking in #107689. Same for the next one. Pull Request resolved: #107710 Approved by: https://github.com/ezyang ghstack dependencies: #107687, #107688
Gather does the same thing, but it's much better supported in the `torch.compile` stack Pull Request resolved: #107711 Approved by: https://github.com/ezyang ghstack dependencies: #107687, #107688, #107710
Stack from ghstack (oldest at bottom):
In this case, we copy, but this is part of the set of divergences
described in Quansight-Labs/numpy_pytorch_interop#73.
This does not work with dynamic shapes, but it's not clear to me what
would be the best fix
cc @mruberry @rgommers @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov