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

AutogradMeta is nullptr for non-differentiable tensors on creation #128746

Open
wants to merge 15 commits into
base: gh/soulitzer/308/base
Choose a base branch
from
Open

AutogradMeta is nullptr for non-differentiable tensors on creation #128746

wants to merge 15 commits into from

Conversation

soulitzer
Copy link
Contributor

@soulitzer soulitzer commented Jun 14, 2024 •
edited
Loading

Stack from ghstack (oldest at bottom):

After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:

  • TensorImpl's autograd_meta_ field are only set in:
    • set_requires_grad (only non-null when requires_grad=True)
    • set_autograd_meta is called in...
      • materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
      • variable_data (sets to nullptr)
      • make_variable_differentiable_view (requires differentiable dtype)
      • make_variable_non_differentiable_view (?)
      • make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in detach_.

@soulitzer soulitzer requested a review from albanD as a code owner June 14, 2024 21:29
@soulitzer soulitzer mentioned this pull request Jun 14, 2024
Closed
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jun 14, 2024 •
edited
Loading

🔗 Helpful Links

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

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

❌ 5 New Failures, 17 Unrelated Failures

As of commit ba5c404 with merge base 93a33bf (image):

NEW FAILURES - The following jobs have failed:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

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

soulitzer added a commit that referenced this pull request Jun 14, 2024
@soulitzer
Ensure AutogradMeta is nullptr for non-differentiable tensors
eb95edb 
ghstack-source-id: dd5c869c8167b10d1059a3788260f426b5cfa0fa
Pull Request resolved: #128746
@soulitzer
Update on "Ensure AutogradMeta is nullptr for non-differentiable tens…
daba05e 
…ors"


After this PR, I expect AutogradMeta to ALWAYS be nullptr for tensor of non-differentiable dtype:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (sets to nullptr)
     - make_variable (only non-null when requires_grad=True)

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Jun 17, 2024
@soulitzer
Ensure AutogradMeta is nullptr for non-differentiable tensors
2cd3ca1 
ghstack-source-id: 85a8c93d8591ecd16183db320416a2e3e9a3a795
Pull Request resolved: #128746
@soulitzer soulitzer changed the title Ensure AutogradMeta is nullptr for non-differentiable tensors AutogradMeta is nullptr for non-differentiable tensors on creation Jun 18, 2024
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
518edde 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
4334063 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
a1b0328 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
9f7be11 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Jun 18, 2024
@soulitzer
Ensure AutogradMeta is nullptr for non-differentiable tensors
65b1d16 
ghstack-source-id: 518e5940fcf7e63237ec2724104b595b56e332f4
Pull Request resolved: #128746
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
9fa4db5 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
02b0f6b 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
4ca4ecd 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Jun 24, 2024
@soulitzer
Ensure AutogradMeta is nullptr for non-differentiable tensors
c527d1c 
ghstack-source-id: a2b620818a08d94a9974882114c7ec31b02b3faa
Pull Request resolved: #128746
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
f44f0bf 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
24fa4fc 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
a8ecd0c 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
@soulitzer
Update on "AutogradMeta is nullptr for non-differentiable tensors on …
ba5c404 
…creation"


After this PR, I expect AutogradMeta to be nullptr for tensor of non-differentiable dtype for more cases:
- TensorImpl's autograd_meta_ field are only set in:
   - set_requires_grad  (only non-null when requires_grad=True)
   - set_autograd_meta is called in...
     - materialize_autograd_meta (only set when accessing autograd APIs like add/remove hook, set name, create_gradient_edge)
     - variable_data (sets to nullptr)
     - make_variable_differentiable_view (requires differentiable dtype)
     - make_variable_non_differentiable_view (?)
     - make_variable (only non-null when requires_grad=True)

Views performed on tensor of non-differentiable dtype still need to be recorded, leading to non-nullptr AutogradMeta.

Previously materialize_autograd_meta was also unconditionally called in `detach_`. 

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Jun 25, 2024
@soulitzer
Ensure AutogradMeta is nullptr for non-differentiable tensors
aa9cc96 
ghstack-source-id: 8ef2b543026a2aba9d74a65dc89f060e03051eb6
Pull Request resolved: #128746
@soulitzer
Copy link
Contributor Author

Some failures may be legit - the traced out ops may be different than before

Doing some measuring, there does not seem to be a significant difference in runtime and instruction count.

click to see script 
import torch
from torch.utils.benchmark import Timer
from torch.utils.benchmark import Measurement


setup="""
"""

stmt="""
a = torch.empty((0,), requires_grad=False)
"""

timer = Timer(stmt, setup)

m = timer.blocked_autorange(min_run_time=1)
print(m)
print(timer.collect_callgrind(100))

@github-actions GitHub Actions
Copy link

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albanD albanD Awaiting requested review from albanD albanD is a code owner

Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@soulitzer