[Dynamo] support isinstance(...) check for type tuple
#146984
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/146984
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e02bcaa with merge base 1677a31 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ghstack-source-id: f641e21 Pull Request resolved: pytorch#146984
| isinstance_type_tuple: tuple[type, ...] | ||
| if isinstance(isinstance_type, type) or callable( | ||
| # E.g. isinstance([], typing.Sequence) | ||
| getattr(isinstance_type, "__instancecheck__", None) |
There was a problem hiding this comment.
Couldn't this call arbirary code at compile time? Do we need to trace through it?
There was a problem hiding this comment.
Couldn't this call arbirary code at compile time?
Yes, it can. But I'm not sure we need to support this. The previous implementation before this PR already can run an arbitrary __subclasscheck__ method. We did not trace through it.
Do we need to trace through it?
Trace through this can have intensive overheads of isinstance(...) which is really common in user code. Also, most __subclasscheck__ can be constant fold through, which degenerates to issubclass(vt1.as_python_constant(), vt2.as_python_constant()).
jansel
left a comment
jansel
left a comment
There was a problem hiding this comment.
Looks like the previous code had the same issue.
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
ghstack-source-id: c8f8fd5 Pull Request resolved: pytorch#146984
Stack from ghstack (oldest at bottom):
isinstance(...)check for polyfilled class #146921isinstance(...)check for type tuple #146984cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames