Fix NativeFunctions.h for c10-full ops#46090
Fix NativeFunctions.h for c10-full ops#46090smessmer wants to merge 5 commits intogh/smessmer/259/basefrom
Conversation
Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/) [ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit e15b09d (more details on the Dr. CI page):
Extra GitHub checks: 1 failed
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group. This comment has been revised 13 times. |
tools/codegen/api/dispatcher.py
Outdated
| else: | ||
| assert local.use_c10_dispatcher() is UseC10Dispatcher.with_codegenerated_unboxing_wrapper | ||
| la = legacy_dispatcher.argument(a) | ||
| assert len(la) == 1, "TensorOptions arguments not supported" |
There was a problem hiding this comment.
IIUC, the real invariant here is that when local.use_c10_dispatcher() is UseC10Dispatcher.with_codegenerated_unboxing_wrapper then legacy_dispatcher.argument(a) is guaranteed to return a single element; it has nothing to do with TensorOptions arguments not being supported.
Although this is ok to leave up to personal preference, I think a better way to handle the problem here is to split legacy_dispatcher.argument into two functions: one that is guaranteed to return only a single LegacyDispatcherArgument (but is only valid for with_codegenerated_unboxing_wrapper), and then another one which is valid in all cases but might return a sequence.
| name='pin_memory', | ||
| default='{}', | ||
| argument=a, | ||
| )] |
There was a problem hiding this comment.
And now we pray that this doesn't need to be packed (in the same way CppArguments need it)......
Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/) [ghstack-poisoned]
Codecov Report
@@ Coverage Diff @@
## gh/smessmer/259/base #46090 +/- ##
=======================================================
Coverage ? 68.24%
=======================================================
Files ? 410
Lines ? 53639
Branches ? 0
=======================================================
Hits ? 36607
Misses ? 17032
Partials ? 0 Continue to review full report at Codecov.
|
Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/) [ghstack-poisoned]
Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/) [ghstack-poisoned]
Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/) [ghstack-poisoned]
|
This pull request has been merged in 4534bf5. |
|
This pull request has been merged in 4534bf5. |
|
"Fix" -- what's broken? |
Pull Request resolved: pytorch/pytorch#46090 ghstack-source-id: 114269272 Differential Revision: [D24219942](https://our.internmc.facebook.com/intern/diff/D24219942/)
Stack from ghstack:
Differential Revision: D24219942