Use TensorMethods.cpp by ilia-cher · Pull Request #37639 · pytorch/pytorch

ilia-cher · 2020-05-01T02:40:04Z

Stack from ghstack:

RecordFunction in Dispatcher #37587 RecordFunction in Dispatcher
Use TensorMethods.cpp #37639 Use TensorMethods.cpp

Summary:
Changing TensorMethods.h to .cpp
Necessary to avoid incomplete types in dispatcher

Test Plan:
CI

Differential Revision: D21374247

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

dr-ci · 2020-05-01T02:50:02Z

💊 CI failures summary and remediations

As of commit 7258615 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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.

See how this bot performed.

This comment has been revised 479 times.

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

dzhulgakov · 2020-05-05T06:31:53Z

Hm, any concerns with overhead due to moving away from the inline methods?

ezyang · 2020-05-05T18:10:11Z


  template <typename T>
-  T * data_ptr() const;
+  TORCH_API T * data_ptr() const;


Hmm, that's weird. The API annotation on the class itself should have been sufficient. What's going on here?

err, will update; I was debugging a windows build error

ezyang · 2020-05-05T18:10:32Z

  c10::intrusive_ptr<TensorImpl, UndefinedTensorImpl> impl_;
 };

+TORCH_API int64_t get_device(Tensor self);


Pretty sure this should be CAFFE2_API

as I understand TORCH_API is defined as CAFFE2_API in WindowsTorchApiMacro.h

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

facebook-github-bot · 2020-06-01T02:09:23Z

This pull request has been merged in 68e62b9.

ezyang · 2020-06-01T15:08:03Z

This broke mobile code analysis. It will not be too easy to fix, we should talk about this with @ljk53 in composability.

ilia-cher · 2020-06-01T16:39:49Z

This broke mobile code analysis. It will not be too easy to fix, we should talk about this with @ljk53 in composability.

@ezyang @ljk53 why was all of CI green? how can I repro?

ljk53 · 2020-06-01T16:57:36Z

mobile

It didn't break because the CI only runs on landed PR. I made it so because there is another e2e CI also covering most part of mobile code analysis. There is only a small unit test that e2e CI doesn't run, which caused the failure.

This is the error message on failed CI job:

Jun 01 15:31:43 In file included from /var/lib/jenkins/workspace/test/mobile/op_deps/simple_ops.cpp:1:
Jun 01 15:31:43 /var/lib/jenkins/workspace/test/mobile/op_deps/simple_ops.h:22:52: error: no member named 'singleton' in 'c10::DispatchKey'
Jun 01 15:31:43   static c10::OperatorHandle op = c10::Dispatcher::singleton()
Jun 01 15:31:43                                   ~~~~~~~~~~~~~~~~~^
Jun 01 15:31:43 /var/lib/jenkins/workspace/test/mobile/op_deps/simple_ops.h:24:15: error: no member named 'Dispatcher' in namespace 'c10'; did you mean 'DispatchKey'?
Jun 01 15:31:43   return c10::Dispatcher::singleton().call<Tensor, const Tensor&>(
Jun 01 15:31:43          ~~~~~^~~~~~~~~~
Jun 01 15:31:43               DispatchKey

I think we need change it accordingly: https://github.com/pytorch/pytorch/blob/master/test/mobile/op_deps/simple_ops.h

You can repeat the test locally using:

https://github.com/pytorch/pytorch/blob/master/tools/code_analyzer/build.sh#L12

ezyang · 2020-06-01T18:12:44Z

You can force all of master ci to run by pushing to ci-all/blah branch on pytorch/pytorch

Summary: see #37639 Test Plan: #37639 [ghstack-poisoned]

Summary: see #37639 Test Plan: #37639 ghstack-source-id: d19b4cb Pull Request resolved: #39385

Summary: see #37639 Test Plan: #37639 Differential Revision: [D21833287](https://our.internmc.facebook.com/intern/diff/D21833287) [ghstack-poisoned]

Summary: see #37639 Test Plan: #37639 ghstack-source-id: 5971b33 Pull Request resolved: #39385

Summary: see #37639 Test Plan: #37639 Differential Revision: [D21833287](https://our.internmc.facebook.com/intern/diff/D21833287) [ghstack-poisoned]

Summary: see #37639 Test Plan: #37639 ghstack-source-id: 561293e Pull Request resolved: #39385

Summary: Pull Request resolved: #39385 see #37639 Test Plan: #37639 Imported from OSS Differential Revision: D21833287 fbshipit-source-id: 9928d3f4122903d0de67ad312e349352d5f5c19c

Summary: Pull Request resolved: pytorch/pytorch#37639 Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Imported from OSS checked mobile size, no change, small reduction in size in fbios fbios: Succeeded Change in Download Size for arm64 + 3x assets variation: -18.2 KiB Change in Uncompressed Size for arm64 + 3x assets variation: -8.8 KiB reran benchmark, no stat. significant difference buck run mode/opt caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads:benchmark_torchscript_model -- --model_file caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt --num_runs 3 ╷ @ 68592d0d 41 minutes ago iliacher D21374247 ╭─╯ Use TensorMethods.cpp Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1729113760 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/3867976782 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2782186766 hg prev @ 7f501b42 Thursday at 14:26 bvaughan D21764704 ╷ short-circuit pow for complex 1 and 0 exponents Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2155256332 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1802057074 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/4119590830 Differential Revision: D21374247 fbshipit-source-id: 076964415079cf84fb57f1f7b43d087afed86e1d

Summary: Pull Request resolved: pytorch/pytorch#39385 see pytorch/pytorch#37639 Test Plan: pytorch/pytorch#37639 Imported from OSS Differential Revision: D21833287 fbshipit-source-id: 9928d3f4122903d0de67ad312e349352d5f5c19c

Summary: Pull Request resolved: pytorch/pytorch#37639 Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Imported from OSS checked mobile size, no change, small reduction in size in fbios fbios: Succeeded Change in Download Size for arm64 + 3x assets variation: -18.2 KiB Change in Uncompressed Size for arm64 + 3x assets variation: -8.8 KiB reran benchmark, no stat. significant difference buck run mode/opt caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads:benchmark_torchscript_model -- --model_file caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt --num_runs 3 ╷ @ 68592d0d 41 minutes ago iliacher D21374247 ╭─╯ Use TensorMethods.cpp Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1729113760 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/3867976782 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2782186766 hg prev @ 7f501b42 Thursday at 14:26 bvaughan D21764704 ╷ short-circuit pow for complex 1 and 0 exponents Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2155256332 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1802057074 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/4119590830 Differential Revision: D21374247 fbshipit-source-id: 076964415079cf84fb57f1f7b43d087afed86e1d

Summary: Pull Request resolved: pytorch/pytorch#39385 see pytorch/pytorch#37639 Test Plan: pytorch/pytorch#37639 Imported from OSS Differential Revision: D21833287 fbshipit-source-id: 9928d3f4122903d0de67ad312e349352d5f5c19c

Summary: Pull Request resolved: pytorch#37639 Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Imported from OSS checked mobile size, no change, small reduction in size in fbios fbios: Succeeded Change in Download Size for arm64 + 3x assets variation: -18.2 KiB Change in Uncompressed Size for arm64 + 3x assets variation: -8.8 KiB reran benchmark, no stat. significant difference buck run mode/opt caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads:benchmark_torchscript_model -- --model_file caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt --num_runs 3 ╷ @ 68592d0d 41 minutes ago iliacher D21374247 ╭─╯ Use TensorMethods.cpp Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1729113760 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/3867976782 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2782186766 hg prev @ 7f501b42 Thursday at 14:26 bvaughan D21764704 ╷ short-circuit pow for complex 1 and 0 exponents Created 3 benchmark runs on aibench for caffe2/caffe2/fb/high_perf_models/pytorch/benchmark_framework_overheads/addmodule.pt. Links to the results: * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/2155256332 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/1802057074 * Adhoc run: https://our.intern.facebook.com/intern/aibench/details/4119590830 Differential Revision: D21374247 fbshipit-source-id: 076964415079cf84fb57f1f7b43d087afed86e1d

Summary: Pull Request resolved: pytorch#39385 see pytorch#37639 Test Plan: pytorch#37639 Imported from OSS Differential Revision: D21833287 fbshipit-source-id: 9928d3f4122903d0de67ad312e349352d5f5c19c

Use TensorMethods.cpp

740d007

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

This was referenced May 1, 2020

Make RecordFunction callbacks thread local and modernize interface #37491

Closed

Move RecordFunction into ATen #37548

Closed

RecordFunction in Dispatcher #37587

Closed

ilia-cher added 2 commits May 1, 2020 02:05

Update on "Use TensorMethods.cpp"

d5978fc

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

5b88202

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

ilia-cher requested a review from apaszke as a code owner May 1, 2020 23:22

ilia-cher added 3 commits May 1, 2020 18:02

Update on "Use TensorMethods.cpp"

08bc699

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

b7a00e4

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

2cbff6d

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

ilia-cher mentioned this pull request May 3, 2020

Propagate TLS callbacks with ThreadLocalState #37745

Closed

Update on "Use TensorMethods.cpp"

ddd07ba

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

ilia-cher mentioned this pull request May 3, 2020

Make profiler thread local #36291

Closed

ilia-cher added 8 commits May 3, 2020 18:22

Update on "Use TensorMethods.cpp"

406a82e

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

3617152

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

3d6d7b7

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

a12030c

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

bd6ca83

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

93900e1

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

8a449f4

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

e6070fc

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

This was referenced May 4, 2020

Move ThreadLocalDebugInfo to c10 #37774

Closed

Memory profiling #37775

Closed

ilia-cher added 3 commits May 4, 2020 12:37

Update on "Use TensorMethods.cpp"

a4b393c

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

87f8c47

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

0a14d99

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

ezyang reviewed May 5, 2020

View reviewed changes

ilia-cher added 4 commits May 28, 2020 14:31

Update on "Use TensorMethods.cpp"

bfe410f

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

39f39be

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

23b5b66

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

Update on "Use TensorMethods.cpp"

7258615

Summary: Changing TensorMethods.h to .cpp Necessary to avoid incomplete types in dispatcher Test Plan: CI Differential Revision: [D21374247](https://our.internmc.facebook.com/intern/diff/D21374247) [ghstack-poisoned]

facebook-github-bot closed this in 68e62b9 Jun 1, 2020

facebook-github-bot added the merged label Jun 1, 2020

ilia-cher mentioned this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp #39385

Closed

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp

fa2cf6a

Summary: see #37639 Test Plan: #37639 [ghstack-poisoned]

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp

c08c1ab

Summary: see #37639 Test Plan: #37639 ghstack-source-id: d19b4cb Pull Request resolved: #39385

ilia-cher mentioned this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp #39386

Closed

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

Update on "[resubmit] Use TensorMethods.cpp"

fed7420

Summary: see #37639 Test Plan: #37639 Differential Revision: [D21833287](https://our.internmc.facebook.com/intern/diff/D21833287) [ghstack-poisoned]

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp

2bf8c6d

Summary: see #37639 Test Plan: #37639 ghstack-source-id: 5971b33 Pull Request resolved: #39385

ilia-cher mentioned this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp #39387

Closed

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

Update on "[resubmit] Use TensorMethods.cpp"

44e72ec

Summary: see #37639 Test Plan: #37639 Differential Revision: [D21833287](https://our.internmc.facebook.com/intern/diff/D21833287) [ghstack-poisoned]

ilia-cher pushed a commit that referenced this pull request Jun 2, 2020

[resubmit] Use TensorMethods.cpp

3a5b2a1

Summary: see #37639 Test Plan: #37639 ghstack-source-id: 561293e Pull Request resolved: #39385

facebook-github-bot deleted the gh/ilia-cher/65/head branch June 4, 2020 14:16

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use TensorMethods.cpp#37639

Use TensorMethods.cpp#37639
ilia-cher wants to merge 84 commits intogh/ilia-cher/65/basefrom
gh/ilia-cher/65/head

ilia-cher commented May 1, 2020 •

edited

Loading

Uh oh!

dr-ci Bot commented May 1, 2020 •

edited

Loading

Uh oh!

dzhulgakov commented May 5, 2020

Uh oh!

ezyang May 5, 2020

Uh oh!

ilia-cher May 5, 2020

Uh oh!

ezyang May 5, 2020

Uh oh!

ilia-cher May 6, 2020

Uh oh!

facebook-github-bot commented Jun 1, 2020

Uh oh!

ezyang commented Jun 1, 2020

Uh oh!

ilia-cher commented Jun 1, 2020

Uh oh!

ljk53 commented Jun 1, 2020

Uh oh!

ezyang commented Jun 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ilia-cher commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci Bot commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

dzhulgakov commented May 5, 2020

Uh oh!

ezyang May 5, 2020

Choose a reason for hiding this comment

Uh oh!

ilia-cher May 5, 2020

Choose a reason for hiding this comment

Uh oh!

ezyang May 5, 2020

Choose a reason for hiding this comment

Uh oh!

ilia-cher May 6, 2020

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jun 1, 2020

Uh oh!

ezyang commented Jun 1, 2020

Uh oh!

ilia-cher commented Jun 1, 2020

Uh oh!

ljk53 commented Jun 1, 2020

Uh oh!

ezyang commented Jun 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ilia-cher commented May 1, 2020 •

edited

Loading

dr-ci Bot commented May 1, 2020 •

edited

Loading