ONNX Export Scatter #18543

lara-hdr · 2019-03-27T21:35:35Z

No description provided.

facebook-github-bot

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

houseroad

Could you add test cases?

lara-hdr · 2019-03-29T22:06:48Z

Caffe2 does not have an operator Scatter, but has ScatterAssign defined in utility_ops (that enforces in place operations). Will I have to add a new operator Scatter?

houseroad · 2019-03-29T22:13:02Z

Yeah, they are quite different. I would suggest to add a new Scatter operator then.

houseroad · 2019-04-02T21:58:58Z

cc: @BIT-silence for the scatter op part.

caffe2/operators/utility_ops.h

houseroad · 2019-04-19T04:25:30Z

@lara-hdr could you address the comments from @BIT-silence ?

…aidar/onnx_export_scatter

facebook-github-bot

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

…aidar/onnx_export_scatter

…a-hdr/pytorch into lahaidar/onnx_export_scatter

facebook-github-bot

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

lara-hdr · 2019-04-30T18:22:16Z

@houseroad conflicts are solved in this PR :)

lara-hdr · 2019-05-08T16:27:47Z

@houseroad, comments on this PR?

…aidar/onnx_export_scatter

lara-hdr · 2019-05-20T02:23:25Z

@houseroad updates on this?

facebook-github-bot

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

caffe2/operators/utility_ops.h

xiaomengy · 2019-05-20T23:55:07Z

caffe2/operators/utility_ops.h

+
+  bool RunOnDevice() override {
+    return DispatchHelper<TensorTypes<int32_t, int64_t>>::call(
+        this, this->template Input<Tensor>(INDICES, CPU));


Should we fix CPU here?

I would suggest to provide better information here, if the tensor is on some devices other than CPU. To explicitly tell them, Scatter only supports CPU at the moment. :-)

If it is a CPU only op, why not remove template Context?

added a check on the device type, and changed Operator to Operator.

houseroad · 2019-05-21T00:41:25Z

caffe2/operators/utility_ops.h

+        OP_SINGLE_ARG(int, "axis", axis_, 1) {
+  }
+
+  virtual ~ScatterOp() noexcept {}


In file included from caffe2/caffe2/operators/utility_ops.cc:1:
caffe2/caffe2/operators/utility_ops.h:752:11: warning: '~ScatterOp' overrides a destructor but is not marked 'override'

virtual ~ScatterOp() noexcept {} ^ caffe2/c10/util/Type.h:19:56: note: in instantiation of template class 'caffe2::ScatterOp<caffe2::CPUContext>' requested here static const auto& name = *(new std::string(demangle(typeid(T).name()))); ^ caffe2/caffe2/operators/utility_ops.cc:54:1: note: in instantiation of function template specialization 'c10::demangle_type<caffe2::ScatterOp<caffe2::CPUContext> >' requested here REGISTER_CPU_OPERATOR(Scatter, ScatterOp<CPUContext>); ^ caffe2/caffe2/core/operator.h:1257:3: note: expanded from macro 'REGISTER_CPU_OPERATOR' C10_REGISTER_CLASS(CPUOperatorRegistry, name, __VA_ARGS__) ^ caffe2/c10/util/Registry.h:291:3: note: expanded from macro 'C10_REGISTER_CLASS' C10_REGISTER_TYPED_CLASS(RegistryName, #key, __VA_ARGS__) ^ caffe2/c10/util/Registry.h:251:14: note: expanded from macro 'C10_REGISTER_TYPED_CLASS' ::c10::demangle_type<__VA_ARGS__>()); ^ caffe2/caffe2/core/operator.h:751:3: note: overridden virtual function is here ~Operator() noexcept override {} ^

please address this comment as well, thanks.

…aidar/onnx_export_scatter

facebook-github-bot

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

facebook-github-bot

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

houseroad

LGTM, thanks.

houseroad · 2019-05-21T22:03:23Z

torch/onnx/symbolic_opset9.py

+
+
+@parse_args('v', 'i', 'v', 'v')
+def scatter_add(g, self, dim, index, src):


Add a test?

facebook-github-bot · 2019-05-22T20:38:06Z

@houseroad merged this pull request in 8d7a025.

houseroad · 2019-05-22T20:43:13Z

Feel free to add a test for scatter_add in a different PR, since this one is already closed. :-)

ONNX Export Scatter

b460381

facebook-github-bot reviewed Mar 28, 2019

View reviewed changes

houseroad reviewed Mar 28, 2019

View reviewed changes

add caffe2 scatter op and test

49d4b7d

houseroad requested a review from xiaomengy April 2, 2019 21:58

xiaomengy reviewed Apr 2, 2019

View reviewed changes

Lara and others added 4 commits April 22, 2019 10:44

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

62b8fd3

…aidar/onnx_export_scatter

update caffe2 scatter op

e2932bc

update caffe2 scatter op

9dc9841

Update utility_ops.h

0002865

facebook-github-bot reviewed Apr 23, 2019

View reviewed changes

Lara added 3 commits April 29, 2019 18:08

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

0d3936a

…aidar/onnx_export_scatter

Merge branch 'lahaidar/onnx_export_scatter' of https://github.com/lar…

db90da4

…a-hdr/pytorch into lahaidar/onnx_export_scatter

remove warning

a3b4bbd

facebook-github-bot reviewed Apr 30, 2019

View reviewed changes

Merge branch 'master' into lahaidar/onnx_export_scatter

f2b7dbb

Lara added 2 commits May 14, 2019 15:35

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

0fffadb

…aidar/onnx_export_scatter

resolve conflict after merge

9409788

pytorchbot added caffe2 module: onnx Related to torch.onnx labels May 14, 2019

facebook-github-bot reviewed May 20, 2019

View reviewed changes

xiaomengy reviewed May 20, 2019

View reviewed changes

houseroad reviewed May 21, 2019

View reviewed changes

lara-hdr added 2 commits May 21, 2019 01:02

update scatter op

20db68c

Merge branch 'master' of https://github.com/lara-hdr/pytorch into lah…

2dac820

…aidar/onnx_export_scatter

facebook-github-bot reviewed May 21, 2019

View reviewed changes

add device check in scatterop

4a61c7d

facebook-github-bot reviewed May 21, 2019

View reviewed changes

houseroad approved these changes May 21, 2019

View reviewed changes

houseroad reviewed May 21, 2019

View reviewed changes

torch/onnx/symbolic_opset9.py

@parse_args('v', 'i', 'v', 'v')

def scatter_add(g, self, dim, index, src):

Copy link

Member

houseroad May 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test?

facebook-github-bot closed this in 8d7a025 May 22, 2019

facebook-github-bot added the merged label May 22, 2019

ezyang added the open source label Jun 24, 2019



		@parse_args('v', 'i', 'v', 'v')
		def scatter_add(g, self, dim, index, src):

ONNX Export Scatter #18543

ONNX Export Scatter #18543

Uh oh!

Conversation

lara-hdr commented Mar 27, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

lara-hdr commented Mar 29, 2019

Uh oh!

houseroad commented Mar 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

houseroad commented Apr 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

houseroad commented Apr 19, 2019

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!

lara-hdr commented Apr 30, 2019

Uh oh!

lara-hdr commented May 8, 2019

Uh oh!

lara-hdr commented May 20, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xiaomengy May 20, 2019

Choose a reason for hiding this comment

Uh oh!

houseroad May 21, 2019

Choose a reason for hiding this comment

Uh oh!

xiaomengy May 21, 2019

Choose a reason for hiding this comment

Uh oh!

lara-hdr May 21, 2019

Choose a reason for hiding this comment

Uh oh!

houseroad May 21, 2019

Choose a reason for hiding this comment

Uh oh!

houseroad May 21, 2019

Choose a reason for hiding this comment

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!

houseroad left a comment

Choose a reason for hiding this comment

Uh oh!

houseroad May 21, 2019

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented May 22, 2019

houseroad commented Mar 29, 2019 •

edited

Loading