Skip to content
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

Add a deprecation notice to the Profile Tensorboard tutorial #3128

Merged
merged 11 commits into from
Oct 31, 2024
Merged

Add a deprecation notice to the Profile Tensorboard tutorial #3128

merged 11 commits into from
Oct 31, 2024

Conversation

svekars
Copy link
Contributor

@svekars svekars commented Oct 30, 2024 •
edited
Loading

  • Add a note about deprecation of the profiler use case in the Profiler Tensorboard tutorial and suggest using Perfetto or Chrome trace instead.

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Oct 30, 2024 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 5c8915b with merge base 8d19c5c (image):
💚 Looks good so far! There are no failures yet. 💚

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

sraikund16
sraikund16 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@sraikund16 sraikund16 left a comment

Choose a reason for hiding this comment

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

Seems fine from the profiler side since we use perfetto/chrome tracer now. Please check in with other teams if they advertise TB before merging though!

janeyx99
janeyx99 reviewed Oct 31, 2024
View reviewed changes
Copy link
Contributor

@janeyx99 janeyx99 left a comment

Choose a reason for hiding this comment

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

So it looks like only the profiler integration is deprecated...let's not touch the other stuff until we're confirmed

intermediate_source/tensorboard_profiler_tutorial.rst Outdated

This tutorial has been deprecated.

Redirecting in 3 seconds...
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of redirecting this one, we should add a "use the perfetto with the trace.json instead" comment, similar to the note in https://pytorch.org/tutorials/advanced/cpp_extension.html
image

The warning I'd imagine would say:

Tensorboard integration with the profiler is now deprecated in favor of using perfetto or the chrome trace for viewing trace.jsons. After generating a trace, please drag the trace.json into perfetto or chrome://tracing to visualize your profile.

intermediate_source/tensorboard_tutorial.rst Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep this one for now, we don't know if Tensorboard UI is deprecated

recipes_source/recipes/tensorboard_with_pytorch.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with keeping this one! THough....who even does work on Tensorboard now....

index.rst Outdated
@@ -88,13 +88,6 @@ Welcome to PyTorch Tutorials
:link: beginner/nn_tutorial.html
:tags: Getting-Started

.. customcarditem::
Copy link
Contributor

Choose a reason for hiding this comment

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

keep

index.rst Outdated
@@ -956,7 +941,6 @@ Additional Resources
beginner/pytorch_with_examples
beginner/nn_tutorial
intermediate/nlp_from_scratch_index
intermediate/tensorboard_tutorial
Copy link
Contributor

Choose a reason for hiding this comment

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

keep?

index.rst Outdated
@@ -942,7 +928,6 @@ Additional Resources
beginner/introyt/tensors_deeper_tutorial
beginner/introyt/autogradyt_tutorial
beginner/introyt/modelsyt_tutorial
beginner/introyt/tensorboardyt_tutorial
Copy link
Contributor

Choose a reason for hiding this comment

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

keep?

beginner_source/introyt/tocyt.txt Outdated
@@ -2,6 +2,5 @@
2. `Introduction to PyTorch Tensors <introyt/tensors_deeper_tutorial.html>`_
3. `The Fundamentals of Autograd <introyt/autogradyt_tutorial.html>`_
4. `Building Models with PyTorch <introyt/modelsyt_tutorial.html>`_
5. `PyTorch TensorBoard Support <introyt/tensorboardyt_tutorial.html>`_
Copy link
Contributor

Choose a reason for hiding this comment

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

keep

beginner_source/introyt/autogradyt_tutorial.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we keep all the yt ones just cuz removing one would be a bit incoherent

@svekars svekars added the skip-link-check Will allow you to skip linkcheck on a PR. Should only should be used when a link can't be fixed. label Oct 31, 2024
janeyx99
janeyx99 reviewed Oct 31, 2024
View reviewed changes
Copy link
Contributor

@janeyx99 janeyx99 left a comment

Choose a reason for hiding this comment

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

Can you change the PR title to reflect the change too?

janeyx99
janeyx99 approved these changes Oct 31, 2024
View reviewed changes
Copy link
Contributor

@janeyx99 janeyx99 left a comment

Choose a reason for hiding this comment

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

Looks fine otherwise, one last thing to keep

@svekars svekars changed the title Deprecate Tensorboard tutorials Add a deprecation notice to the Profile Tensorboard tutorial Oct 31, 2024
@svekars svekars merged commit 54273de into main Oct 31, 2024
18 checks passed
@svekars svekars deleted the deprecate-tensorboard-tutorials branch October 31, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sraikund16 sraikund16 sraikund16 approved these changes

@janeyx99 janeyx99 janeyx99 approved these changes

Assignees
No one assigned
Labels
cla signed skip-link-check Will allow you to skip linkcheck on a PR. Should only should be used when a link can't be fixed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@svekars @janeyx99 @sraikund16 @facebook-github-bot