Skip to content

Conversation

soulitzer
Copy link
Contributor

@soulitzer soulitzer commented Mar 19, 2025

…d.Function and marked dirty

[ghstack-poisoned]
@soulitzer soulitzer requested a review from albanD as a code owner March 19, 2025 19:19
Copy link

pytorch-bot bot commented Mar 19, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit 3455c3b with merge base e57cdb8 (image):

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

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

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

@soulitzer soulitzer added release notes: autograd release notes category topic: improvements topic category labels Mar 19, 2025
@pytorch pytorch deleted a comment from github-actions bot Mar 19, 2025
@soulitzer soulitzer changed the title Improve error message when intermediate view is returned from autograd.Function and marked dirty Improve error message when view of intermediate is returned from autograd.Function and marked dirty Mar 19, 2025
…d from autograd.Function and marked dirty"



Fixes #149252

[ghstack-poisoned]
…d from autograd.Function and marked dirty"



Fixes #149252

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Mar 19, 2025
…d.Function and marked dirty

ghstack-source-id: 5a60958
Pull Request resolved: #149543
@soulitzer soulitzer requested a review from zou3519 March 24, 2025 18:12
"Only input Tensors that have been mutated should be passed to "
"ctx.mark_dirty().";
// We reach this path in the view of intermediate case
TORCH_CHECK(!var.is_view(), mark_dirty_error_msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the intermediate case result in var being a view?

Copy link
Contributor Author

@soulitzer soulitzer Mar 24, 2025

Choose a reason for hiding this comment

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

a view of a intermediate tensor is still a view because we don't disable ADInplaceOrView

Copy link
Contributor

Choose a reason for hiding this comment

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

What prevents an input from being a view?

Copy link
Contributor Author

@soulitzer soulitzer Mar 24, 2025

Choose a reason for hiding this comment

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

Inputs can be views and that is fine, we should do the usual rebase history stuff. If you mean to ask what happens if you are a view of the input, your view will be a leaf that require_grad being modified inplace and that is handled else-where (the previous branch of the if-else).

This PR is mainly targeting the case where the view was created from an intermediate, in which we are leaf with requires_grad=False and will immediately error (with an internal assert) because this autograd.Function is marked dirty and will call rebase_history on it.

@soulitzer
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 24, 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

@pytorch-bot pytorch-bot bot had a problem deploying to upload-benchmark-results March 24, 2025 20:30 Failure
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results March 24, 2025 20:30 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results March 24, 2025 20:30 Inactive
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / cuda12.1-py3.10-gcc9-sm80 / build

Details for Dev Infra team Raised by workflow job

@soulitzer
Copy link
Contributor Author

@pytorchbot merge

@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

amathewc pushed a commit to amathewc/pytorch that referenced this pull request Apr 17, 2025
…grad.Function and marked dirty (pytorch#149543)

Fixes pytorch#149252
Pull Request resolved: pytorch#149543
Approved by: https://github.com/zou3519
ghstack dependencies: pytorch#149220
@github-actions github-actions bot deleted the gh/soulitzer/355/head branch April 27, 2025 02:19
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 topic: improvements topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants