[jit] Trace in-place ops #14254
[jit] Trace in-place ops #14254
Conversation
facebook-github-bot
added
the
oncall: jit
|
There's a bunch of clang-tidy issues in script/init.cpp https://travis-ci.org/pytorch/pytorch/jobs/457803711 but they don't seem related to your changes |
There was a problem hiding this comment.
@suo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@suo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@apaszke going to land this if you don't have any objection |
| } | ||
|
|
||
| inline void ensureUnique(const char * name, const at::Tensor& tensor) { | ||
| auto& state = getTracingState(); |
There was a problem hiding this comment.
It might be a good idea to rename this function to ensureUniqueIfOutOfPlaced or sth like that. It's not really used outside of auto generated code, but it would clarify its semantics a bit
| output_names_outplace = [arg['name'] for arg in declaration['arguments'] if arg.get('output', False)] | ||
| output_names_inplace = [r['name'] for r in declaration['returns']] | ||
|
|
||
| if output_names_outplace == output_names_inplace: |
There was a problem hiding this comment.
When does that happen? A comment would be nice
tools/autograd/gen_variable_type.py
Outdated
| def rename(trace_name): | ||
| if trace_name in RENAME_TRACE: | ||
| trace_name = RENAME_TRACE[trace_name] | ||
| return trace_name |
There was a problem hiding this comment.
This is just RENAME_TRACE.get(trace_name, trace_name)?
|
I'll try to add some comments explaining the high-level flow. The messiness seems unavoidable if we want to avoid duplication in generated code though (due to how many weird semantic quirks we have between aten and the JIT schema 😢). I'll address the rest of your comments on the fbcode side since I have changes there that I don't want to blow away. |
Summary: This PR adds a `try_outplace` option to the tracer. When `try_outplace` is true, the tracer will attempt to out-of-place ops (similar to how things are done today). When it's false, the correct in-place op is emitted. I made `try_outplace` false by default, but flipped it to true for ONNX export utils. zdevito jamesr66a, anywhere else I should preserve the existing behavior? Pull Request resolved: pytorch#14254 Differential Revision: D13166691 fbshipit-source-id: 2e8735f5d0e355bea784cbdc344eaeedf93c2640
Summary: This PR adds a `try_outplace` option to the tracer. When `try_outplace` is true, the tracer will attempt to out-of-place ops (similar to how things are done today). When it's false, the correct in-place op is emitted. I made `try_outplace` false by default, but flipped it to true for ONNX export utils. zdevito jamesr66a, anywhere else I should preserve the existing behavior? Pull Request resolved: #14254 Reviewed By: eellison Differential Revision: D13166691 Pulled By: suo fbshipit-source-id: ce39fdf73ac39811c55100e567466d53108e856b
5 participants
This PR adds a
try_outplaceoption to the tracer. Whentry_outplaceis true, the tracer will attempt to out-of-place ops (similar to how things are done today). When it's false, the correct in-place op is emitted.I made
try_outplacefalse by default, but flipped it to true for ONNX export utils. @zdevito @jamesr66a, anywhere else I should preserve the existing behavior?