Skip to content

Conversation

More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 12, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86754

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 9 Pending

As of commit e6bde08:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
Copy link
Contributor

@slgong-fb slgong-fb left a comment

Choose a reason for hiding this comment

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

LGTM!

ska::flat_hash_set<storage_id_t> tensor_set;
auto insert_tensor = [&lookup, &tensors, &tensor_set](TensorMetadata& m) {
if (m.impl_ && m.data_) {
const auto id = lookup(m.data_);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we use reuse lookup() anywhere else? otherwise, we can merge it into insert_tensor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a fair point. You're right, but I'm going to leave it simply because #87133 (later in the stack) removes it altogether.

auto id = lookup(m->data_);
tensor_set.insert(id);
tensors.emplace_back(TensorStoragePair{m->impl_, id, m->id_});
if (m.has_value()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this line too. we can merge this check into insert_tensor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's funny you should ask that. I wanted insert_tensor to take c10::optional, but it turns out that T doesn't implicitly convert to c10::optional<T>. And building an overload set with c10::overloaded wasn't really any cleaner.

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 17, 2022
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
@robieta robieta added the release notes: profiler release notes category label Oct 19, 2022
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
Copy link
Member

@aaronenyeshi aaronenyeshi left a comment

Choose a reason for hiding this comment

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

LGTM!

Taylor Robie added 2 commits October 21, 2022 11:42
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

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

[ghstack-poisoned]
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Nov 5, 2022
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

Differential Revision: [D39852885](https://our.internmc.facebook.com/intern/diff/D39852885/)
Pull Request resolved: pytorch#86754
Approved by: https://github.com/slgong-fb, https://github.com/aaronenyeshi
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
More sophisticated profiling will increasingly rely on python tracer to contextualize observed results. This PR adds Tensors which are observed by the python tracer to the identity assignment loop.

Differential Revision: [D39852885](https://our.internmc.facebook.com/intern/diff/D39852885/)
Pull Request resolved: pytorch#86754
Approved by: https://github.com/slgong-fb, https://github.com/aaronenyeshi
@facebook-github-bot facebook-github-bot deleted the gh/robieta/132/head branch June 8, 2023 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request release notes: profiler release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants