8000 [Profiler] Move Kineto activity generation into `collection.cpp` by robieta · Pull Request #80796 · pytorch/pytorch · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Profiler] Move Kineto activity generation into collection.cpp #80796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

[Profiler] Move Kineto activity generation into collection.cpp #80796

wants to merge 6 commits into from

Conversation

robieta
Copy link
@robieta robieta commented Jul 2, 2022 •
edited
Loading

Stack from ghstack (oldest at bottom):

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in profiler_kineto.cpp for now.

Differential Revision: D37406955

[Profiler] Move Kineto activity generation into collection.cpp
c3a0a10 
In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
@robieta robieta requested review from albanD and soulitzer as code owners July 2, 2022 20:41
@facebook-github-bot
Copy link
Contributor
facebook-github-bot commented Jul 2, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures

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

Expand to see more
  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build trunk / ios-12-5-1-x86-64 / build (1/1)

Step: "Run Simulator Tests" (full log | diagnosis details | 🔁 rerun)

2022-07-17T06:17:39.9882300Z RuntimeError: 
2022-07-17T06:17:39.9875930Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/jit/_recursive.py", line 538, in create_script_module_impl
2022-07-17T06:17:39.9876440Z     script_module = torch.jit.RecursiveScriptModule._construct(cpp_module, init_fn)
2022-07-17T06:17:39.9877180Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/jit/_script.py", line 615, in _construct
2022-07-17T06:17:39.9877670Z     init_fn(script_module)
2022-07-17T06:17:39.9878610Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/jit/_recursive.py", line 516, in init_fn
2022-07-17T06:17:39.9879150Z     scripted = create_script_module_impl(orig_value, sub_concrete_type, stubs_fn)
2022-07-17T06:17:39.9879850Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/jit/_recursive.py", line 542, in create_script_module_impl
2022-07-17T06:17:39.9880560Z     create_methods_and_properties_from_stubs(concrete_type, method_stubs, property_stubs)
2022-07-17T06:17:39.9881320Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/jit/_recursive.py", line 393, in create_methods_and_properties_from_stubs
2022-07-17T06:17:39.9881890Z     concrete_type._create_methods_and_properties(property_defs, property_rcbs, method_defs, method_rcbs, method_defaults)
2022-07-17T06:17:39.9882300Z RuntimeError: 
2022-07-17T06:17:39.9882590Z method cannot be used as a value:
2022-07-17T06:17:39.9883210Z   File "/Users/runner/work/_temp/anaconda/lib/python3.9/site-packages/torch/nn/modules/transformer.py", line 470
2022-07-17T06:17:39.9883630Z                 self.norm2.weight,
2022-07-17T06:17:39.9883940Z                 self.norm2.bias,
2022-07-17T06:17:39.9884230Z                 self.linear1.weight,
2022-07-17T06:17:39.9884680Z                 ~~~~~~~~~~~~~~~~~~~ <--- HERE
2022-07-17T06:17:39.9885070Z                 self.linear1.bias,
2022-07-17T06:17:39.9885480Z                 self.linear2.weight,
2022-07-17T06:17:39.9885760Z 
2022-07-17T06:17:40.3313200Z ##[error]Process completed with exit code 1.
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

This was referenced Jul 2, 2022
Closed
Closed
Closed
Closed
Update on "[Profiler] Move Kineto activity generation into `collectio…
1fa5343 
…n.cpp`"

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
This was referenced Jul 3, 2022
Closed
Closed
Update on "[Profiler] Move Kineto activity generation into `collectio…
60305dd 
…n.cpp`"

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
@robieta robieta requested review from chaekit and aaronenyeshi and removed request for albanD and soulitzer July 3, 2022 22:45
Taylor Robie added 2 commits July 6, 2022 10:08
Update on "[Profiler] Move Kineto activity generation into `collectio…
faf4d0f 
…n.cpp`"

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
Update on "[Profiler] Move Kineto activity generation into `collectio…
31733cf 
…n.cpp`"

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
This was referenced Jul 12, 2022
Closed
Closed
Closed
Closed
Update on "[Profiler] Move Kineto activity generation into `collectio…
cb43a52 
…n.cpp`"

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.

Differential Revision: [D37406955](https://our.internmc.facebook.com/intern/diff/D37406955/)

[ghstack-poisoned]
@robieta robieta added ciflow/trunk Trigger trunk jobs on your pull request ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR labels Jul 15, 2022
@robieta
Copy link
Author
robieta commented Jul 16, 2022

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

Hey @robieta.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@robieta robieta added the release notes: profiler release notes category label Jul 16, 2022
@malfet
Copy link
Contributor
malfet commented Jul 17, 2022

@pytorchbot revert -m "Broke OSS iOS builds, see https://hud.pytorch.org/pytorch/pytorch/commit/24e6b60be29d2e53b967809067fff392c0591e53" -c nosignal

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here

@pytorchmergebot
Copy link
Collaborator

@robieta your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Jul 17, 2022
@pytorchmergebot
Revert "[Profiler] Move Kineto activity generation into `collection.c…
037792f 
…pp` (#80796)"

This reverts commit 24e6b60.

Reverted #80796 on behalf of https://github.com/malfet due to Broke OSS iOS builds, see https://hud.pytorch.org/pytorch/pytorch/commit/24e6b60be29d2e53b967809067fff392c0591e53
@malfet malfet reopened this Jul 17, 2022
@malfet
Copy link
Contributor
malfet commented Jul 17, 2022

@pytorchbot merge -f

Revert were in error, not caused by the change in question

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

Hey @robieta.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Jul 18, 2022
@facebook-github-bot
[Profiler] Move Kineto activity generation into collection.cpp (#80796
0b738b5 
)

Summary:
Pull Request resolved: #80796

In order to fold kineto events into TorchTidy, the new collection machinery needs to handle moving events to and from kineto. This PR is one step in that transition. In order to limit the scope of changes, we keep metadata generation in `profiler_kineto.cpp` for now.
ghstack-source-id: 161652343

Test Plan: Existing unit tests.

Reviewed By: mehtanirav

Differential Revision: D37406955

fbshipit-source-id: a875604bc42a4e1b6d4b08859af171a89750a340
@robieta robieta mentioned this pull request Jul 19, 2022
Closed
@janeyx99 janeyx99 mentioned this pull request Jul 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronenyeshi aaronenyeshi aaronenyeshi approved these changes

@chaekit chaekit Awaiting requested review from chaekit

Assignees
No one assigned
Labels
ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR ciflow/trunk Trigger trunk jobs on your pull request cla signed Merged release notes: profiler release notes category Reverted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@robieta @facebook-github-bot @pytorchmergebot @malfet @aaronenyeshi
0