Add a tagged union type that replaces tensor in the interpreter. by zdevito · Pull Request #9368 · pytorch/pytorch

zdevito · 2018-07-11T23:46:06Z

IValue is short for interpreter value. It is used frequently so a short name is important.
This will allow us to implement more non-tensor types in an efficient way and remove
many hacks from the compiler.

This PR is limited. It only introduces IValue and changes interpreter to use it.
Follow up PRs will:

Change the way aten_ops consume non-tensor types so that integer lists,
are no longer represented as Tensors.
Introduce TensorList as a fundamental type and remove all vararg handling in gen_jit_dispatch
Change the compiler to implement math on primitive numbers rather than converting to tensors.

@jamesr66a @apaszke

facebook-github-bot

@zdevito has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IValue is short for interpreter value. It is used frequently so a short name is important. This will allow us to implement more non-tensor types in an efficient way and remove many hacks from the compiler. This PR is limited. It only introduces IValue and changes interpreter to use it. Follow up PRs will: * Change the way aten_ops consume non-tensor types so that integer lists, are no longer represented as Tensors. * Introduce TensorList as a fundamental type and remove all vararg handling in gen_jit_dispatch * Change the compiler to implement math on primitive numbers rather than converting to tensors.

facebook-github-bot

@zdevito has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

apaszke

LGTM. Some suggestions that might make the code nicer, but nothing big.

tools/jit/templates/register_aten_ops.cpp

+std::vector<at::Tensor> toTensors(at::ArrayRef<IValue> ivalues) {
+  return fmap(ivalues, [](const IValue& v) {
+    return v.toTensor();
+  });


torch/csrc/jit/graph_executor.cpp


+// helper to run interpreter on variables until we switch
+// everything to IValue
+inline variable_tensor_list runOneStage(const Code & code, variable_tensor_list inputs) {


torch/csrc/jit/graph_executor.cpp

+inline variable_tensor_list runOneStage(const Code & code, variable_tensor_list inputs) {
+  std::vector<IValue> stack(inputs.begin(), inputs.end());
+  InterpreterState(code).runOneStage(stack);
+  // note: we never unwrapped inputs, because we want autograd to record the trace


torch/csrc/jit/graph_executor.cpp

+  std::vector<IValue> unwrapVariables(variable_tensor_list && list) const {
+    return fmap(list, [](const Variable& v) -> IValue {
+      return v.defined() ? autograd::as_variable_ref(v).detach() : at::Tensor();
+    });


torch/csrc/jit/ivalue.h

+  }
+private:
+  PointerType * pImpl;
+};


torch/csrc/jit/register_prim_ops.cpp

          return [num_inputs](Stack& stack) {
            bool first = true;
-            for (at::Tensor i : last(stack, num_inputs)) {
+            for (IValue i_ : last(stack, num_inputs)) {


torch/csrc/jit/script/compiler.cpp

-  auto list = constant_as<at::IntList>(input);
+  auto list = constant_as<std::vector<int64_t>>(input);
  if(list)
    return std::vector<int64_t>(*list);


torch/csrc/jit/interpreter.h

  // outputs for that stage, suspending the computation.
  // Call this function again continues computation where it left off.
-  void runOneStage(std::vector<at::Tensor> & stack);
+  void runOneStage(std::vector<IValue> & stack);


torch/csrc/jit/test_jit.cpp

+  auto foo2 = std::move(bar);
+  JIT_ASSERT(foo->use_count() == 3);
+  JIT_ASSERT(foo2.isIntList());
+  JIT_ASSERT(bar.isInt());


torch/csrc/jit/test_jit.cpp

+
+  auto move_it = std::move(baz).toIntList();
+  JIT_ASSERT(foo->use_count() == 2);
+  JIT_ASSERT(baz.isInt());


* Adds a None type for default values * Other minor fixes.

facebook-github-bot

@zdevito has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…orch#9368) Summary: IValue is short for interpreter value. It is used frequently so a short name is important. This will allow us to implement more non-tensor types in an efficient way and remove many hacks from the compiler. This PR is limited. It only introduces IValue and changes interpreter to use it. Follow up PRs will: * Change the way aten_ops consume non-tensor types so that integer lists, are no longer represented as Tensors. * Introduce TensorList as a fundamental type and remove all vararg handling in gen_jit_dispatch * Change the compiler to implement math on primitive numbers rather than converting to tensors. jamesr66a apaszke Pull Request resolved: pytorch#9368 Reviewed By: ezyang Differential Revision: D8817598 Pulled By: zdevito fbshipit-source-id: 29dce80611ce5f6384234de9d12a67861d2b112f

zdevito requested review from apaszke, colesbury, ezyang, gchanan and soumith as code owners July 11, 2018 23:46

zdevito force-pushed the pr/interpreter_value branch from c1d2ce4 to 6ed3af6 Compare July 12, 2018 05:18

facebook-github-bot reviewed Jul 12, 2018

View reviewed changes

zdevito force-pushed the pr/interpreter_value branch from 6ed3af6 to 569e13f Compare July 12, 2018 05:41

facebook-github-bot reviewed Jul 12, 2018

View reviewed changes

apaszke approved these changes Jul 13, 2018

View reviewed changes

PR Comments

c1d2413

* Adds a None type for default values * Other minor fixes.

zdevito force-pushed the pr/interpreter_value branch from e0570f9 to c1d2413 Compare July 16, 2018 18:57

facebook-github-bot reviewed Jul 16, 2018

View reviewed changes

facebook-github-bot closed this in 9ed2190 Jul 16, 2018

ezyang added the merged label Jun 26, 2019

Conversation

zdevito commented Jul 11, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

apaszke left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants