Force specialization on INT_LIST #111216
Force specialization on INT_LIST #111216
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/111216
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit fc3b971 with merge base 6aa91c8 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
github-actions
bot
requested review from
SherlockNoMad,
albanD,
antoniojkim,
bdhirsh,
miladm,
voznesenskym and
wconstab
ezyang
added
ciflow/trunk
Follow up on #95479 Fixes #111198 Fixes #111197 Fixes #111188 I can also do this for some other types, will do this stacked on top. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]
Follow up on #95479 Fixes #111198 Fixes #111197 Fixes #111188 I can also do this for some other types, will do this stacked on top. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]
| } | ||
| } else if (torch::is_symint(py::handle(obj))) { | ||
| res[idx] = py::cast<c10::SymInt>(py::handle(obj)) | ||
| .guard_int(__FILE__, __LINE__); |
There was a problem hiding this comment.
More for my own understanding: instead of silently specializing whenever we plumb a symint to an op who's schema specifies int[], wouldn't it be better to just change all of our schemas to take in SymInt[]? (and until then, raising an error makes it easier to identify and fix the places where this happens?)
One reason the guards here are kinda painful is that (I don't think) we get cpp stacktraces in the guard provenance tracking, so it won't be immediately clear to people who sees guards that the guards are coming from our unfixed schemas.
There was a problem hiding this comment.
oh unless this change is purely for BC reasons - in theory we could change all aten schemas today, but I guess for custom ops, we used to specialize by default whereas now (without this PR) we now error by default on custom op schemas that take in int[].
There was a problem hiding this comment.
So what we specifically are guarding against here is when Dynamo thinks an int is dynamic (most commonly because dynamic=True) but it actually is something like dim= and cannot possibly be dynamic.
I think modifying the schemas would not be the right thing to do, because there isn't any situation we will actually support dynamism for real, so it is wasted work. In the early days, we errored instead of specializing to make it easier to catch missing symints in schema, but we now have good logging and good coverage so it is more important to avoid erroring eg as in the linked issues
Follow up on #95479 Fixes #111198 Fixes #111197 Fixes #111188 I can also do this for some other types, will do this stacked on top. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]
Fixes #111200 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #111219 Approved by: https://github.com/Skylion007, https://github.com/bdhirsh ghstack dependencies: #111216
…ntlist (#111642) Fixes #104812. As of #111216, the python arg parser will now guard and cast symints from dynamo into ints when it is forced to (e.g. when we pass a symint to an op that only accepts ints). But the python arg parser also has logic to try to coerce ints into int[] - we need the same logic for symint -> int[]. Pull Request resolved: #111642 Approved by: https://github.com/ezyang, https://github.com/albanD ghstack dependencies: #111553
Follow up on pytorch#95479 Fixes pytorch#111198 Fixes pytorch#111197 Fixes pytorch#111188 Fixes pytorch#111201 Fixes pytorch#111202 I can also do this for some other types, will do this stacked on top. Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#111216 Approved by: https://github.com/voznesenskym
Fixes pytorch#111200 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#111219 Approved by: https://github.com/Skylion007, https://github.com/bdhirsh ghstack dependencies: pytorch#111216
…ntlist (pytorch#111642) Fixes pytorch#104812. As of pytorch#111216, the python arg parser will now guard and cast symints from dynamo into ints when it is forced to (e.g. when we pass a symint to an op that only accepts ints). But the python arg parser also has logic to try to coerce ints into int[] - we need the same logic for symint -> int[]. Pull Request resolved: pytorch#111642 Approved by: https://github.com/ezyang, https://github.com/albanD ghstack dependencies: pytorch#111553
…ntlist (pytorch#111642) Fixes pytorch#104812. As of pytorch#111216, the python arg parser will now guard and cast symints from dynamo into ints when it is forced to (e.g. when we pass a symint to an op that only accepts ints). But the python arg parser also has logic to try to coerce ints into int[] - we need the same logic for symint -> int[]. Pull Request resolved: pytorch#111642 Approved by: https://github.com/ezyang, https://github.com/albanD ghstack dependencies: pytorch#111553
5 participants
Stack from ghstack (oldest at bottom):
Follow up on #95479
Fixes #111198
Fixes #111197
Fixes #111188
Fixes #111201
Fixes #111202
I can also do this for some other types, will do this stacked on top.
Signed-off-by: Edward Z. Yang ezyang@meta.com
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng