Skip to content

Conversation

@soulitzer
Copy link
Contributor

@soulitzer soulitzer commented May 6, 2025

…nt in extending autograd note

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented May 6, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure, 1 Unrelated Failure

As of commit 73394bc with merge base e2c7ae5 (image):

NEW FAILURE - The following job has failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

… is important in extending autograd note"

[ghstack-poisoned]
@soulitzer soulitzer requested a review from albanD as a code owner May 6, 2025 23:35
soulitzer added a commit that referenced this pull request May 6, 2025
…nt in extending autograd note

ghstack-source-id: 7b86a46
Pull Request resolved: #153005
@soulitzer soulitzer added the release notes: autograd release notes category label May 6, 2025
@pytorch pytorch deleted a comment from github-actions bot May 6, 2025
Comment on lines 115 to 124
Saving tensors via ``save_for_backward`` allows the autograd engine to clear
them as soon as the backward computation of the ``autograd.Function`` completes.
(If a tensor is stored directly on ``ctx``
it will unnecessarily remain alive for the lifetime of the autograd graph --
typically until the end of the iteration.) Using ``save_for_backward``
also helps avoid certain reference cycles, (e.g., since the tensor
output of the ``autograd.Function`` itself keeps a reference to the ctx).
Saving via ``save_for_backward`` is also important for compatibility with
features like activation checkpointing and offloading that rely on
:class:`torch.autograd.graph.saved_tensors_hooks`.
Copy link
Contributor

Choose a reason for hiding this comment

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

A possible readability improvement - not sure if I formatted it correctly - been a while since I touched rst.

Suggested change
Saving tensors via ``save_for_backward`` allows the autograd engine to clear
them as soon as the backward computation of the ``autograd.Function`` completes.
(If a tensor is stored directly on ``ctx``
it will unnecessarily remain alive for the lifetime of the autograd graph --
typically until the end of the iteration.) Using ``save_for_backward``
also helps avoid certain reference cycles, (e.g., since the tensor
output of the ``autograd.Function`` itself keeps a reference to the ctx).
Saving via ``save_for_backward`` is also important for compatibility with
features like activation checkpointing and offloading that rely on
:class:`torch.autograd.graph.saved_tensors_hooks`.
Saving tensors via ``save_for_backward``:
1. allows the autograd engine to clear
them as soon as the backward computation of the ``autograd.Function`` completes.
(If a tensor is stored directly on ``ctx``
it will unnecessarily remain alive for the lifetime of the autograd graph --
typically until the end of the iteration.)
2. it helps avoid certain reference cycles, (e.g., since the tensor
output of the ``autograd.Function`` itself keeps a reference to the ctx).
3. it is also important for compatibility with
features like activation checkpointing and offloading that rely on
:class:`torch.autograd.graph.saved_tensors_hooks`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated. Will double check the rendered results

… is important in extending autograd note"


cc stas00 

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request May 7, 2025
…nt in extending autograd note

ghstack-source-id: 1c0c478
Pull Request resolved: #153005
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks!

@soulitzer
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 8, 2025
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / win-vs2022-cpu-py3 / test (default, 1, 3, ephemeral.windows.4xlarge.nonephemeral)

Details for Dev Infra team Raised by workflow job

@soulitzer
Copy link
Contributor Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged while ignoring the following 2 checks: trunk / macos-py3-arm64 / test (default, 3, 3, macos-m1-stable), trunk / win-vs2022-cpu-py3 / test (default, 1, 3, ephemeral.windows.4xlarge.nonephemeral)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #153094

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 Merged release notes: autograd release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants