Make aten::contiguous and device_put no-op | fix(torchlib) #835
Conversation
Make contiguous an no-op because memory format is not a notion in ONNX
justinchuby
added
the
module: torchlib
justinchuby
changed the title
Codecov Report
@@ Coverage Diff @@
## main #835 +/- ##
==========================================
+ Coverage 76.45% 76.47% +0.01%
==========================================
Files 112 112
Lines 13371 13371
Branches 1342 1341 -1
==========================================
+ Hits 10223 10225 +2
+ Misses 2816 2815 -1
+ Partials 332 331 -1
|
| "memory_format value supports 'contiguous_format' or 'preserve_format' only." | ||
| ) | ||
| # ONNX does not have the notion of memory_format. It is always treated as a no-op. | ||
| return op.Identity(self) |
There was a problem hiding this comment.
I'm wondering if we do so, users calling this function will have a wrong misunderstanding that all of formats were processed successfully, which is not right.
If we can handle this op in an earlier phase by exporter, this should be fine, and we'd better leave a comment here.
There was a problem hiding this comment.
I understand the format is an internal representation and does not affect computation in terms of the result? I gathered that from https://pytorch.org/tutorials/intermediate/memory_format_tutorial.html Please feel free to correct me
There was a problem hiding this comment.
@BowenBao do you have more info on this op? Do you think we should filter it out in a fx pass?
There was a problem hiding this comment.
Chatted offline. Handling in fx pass offers a more fundamental solution that asserts correctness, yet it requires much larger effort and targets only edge cases, which does not cut it in terms of priorities. Hence the approach in this PR is preferred.
justinchuby
changed the title
|
@BowenBao PTAL. Thanks! |
| "memory_format value supports 'contiguous_format' or 'preserve_format' only." | ||
| ) | ||
| # ONNX does not have the notion of memory_format. It is always treated as a no-op. | ||
| return op.Identity(self) |
There was a problem hiding this comment.
Chatted offline. Handling in fx pass offers a more fundamental solution that asserts correctness, yet it requires much larger effort and targets only edge cases, which does not cut it in terms of priorities. Hence the approach in this PR is preferred.
Make contiguous & device_put an no-op because memory format or device is not a notion in ONNX.