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.

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

SetVariable in dynamo #103205

Closed
wants to merge 24 commits into from

Conversation

voznesenskym
Copy link
Collaborator

@voznesenskym voznesenskym commented Jun 7, 2023

Set initial

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Jun 7, 2023

🔗 Helpful Links

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

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

❌ 1 New Failure, 1 Unrelated Failure

As of commit fc038f9:

NEW FAILURE - The following job has failed:

UNSTABLE - The following job failed but was 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.

voznesenskym added a commit that referenced this pull request Jun 7, 2023
Set initial

ghstack-source-id: 79873ff77503d7488c3b2f3929560fb5d3d069aa
Pull Request resolved: #103205
@voznesenskym voznesenskym changed the title Set initial [WIP] SetVariable in dynamo Jun 7, 2023
@ezyang ezyang requested a review from jansel June 8, 2023 17:10
Comment on lines 658 to 665
if sys.version_info >= (3, 11):
codegen.foreach(self.items)
return [create_instruction("BUILD_SET", arg=len(self.items))]
else:
create_load_global("set", True)
codegen.foreach(self.items)
return [create_instruction("CALL_FUNCTION", arg=len(self.items))]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't actually need this to be like this, I can simplify this to just build set.

torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved
torch/_dynamo/symbolic_convert.py Outdated Show resolved Hide resolved
Set initial

cc penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Jun 9, 2023
Set initial

ghstack-source-id: 030e4e6d7c5497c9f96ad9d36c94cdfeaf616761
Pull Request resolved: #103205

wip

WIP

WIP
Set initial

cc penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Jun 9, 2023
Set initial

ghstack-source-id: b7950036819956f2f0670b9ea278cda89d43bdff
Pull Request resolved: #103205

wip

WIP

WIP

WIP
@voznesenskym voznesenskym reopened this Jun 9, 2023
@voznesenskym voznesenskym changed the title [WIP] SetVariable in dynamo SetVariable in dynamo Jun 9, 2023
@github-actions github-actions bot requested a review from wconstab June 9, 2023 18:46
@voznesenskym voznesenskym requested a review from jansel June 9, 2023 18:46
Set initial

cc penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy

[ghstack-poisoned]
Comment on lines +879 to +891
if not self.tx.output.export:
# Export has (supposedly) valid cases for fake tensors as inputs here.
# I am not convinced, atm, but out of scope for what this assert was added for (protecting value checks
# in real_value_tensor_positive_aliases in the common case)
assert not isinstance(value, torch._subclasses.fake_tensor.FakeTensor)

if value in self.tx.output.real_value_tensor_positive_aliases:
stored_value = self.tx.output.real_value_tensor_positive_aliases[value]
# TODO(voz): Decently common pattern, refactor at some point.
dup_guard = self._make_dupe_guard(stored_value)
if dup_guard:
stored_value = stored_value.add_guards(self.make_guards(dup_guard))
return stored_value
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should have been on #104921 - but I fat fingered the commit somewhere.

@atalman
Copy link
Contributor

atalman commented Jul 17, 2023

@voznesenskym Looks like there is consistent failure in our CI after this PR was merged:
https://hud.pytorch.org/hud/pytorch/pytorch/main/1?per_page=50&name_filter=nductor_torchbench_dynamic%2C%20

Looks like CUDA oom.

torch.cuda.OutOfMemoryError: CUDA out of memory. Tried to allocate 768.00 MiB. GPU 0 has a total capacty of 21.99 GiB of which 427.06 MiB is free. Process 17682 has 21.56 GiB memory in use. Of the allocated memory 20.96 GiB is allocated by PyTorch, and 282.55 MiB is reserved by PyTorch but unallocated. If reserved but unallocated memory is large try setting max_split_size_mb to avoid fragmentation.  See documentation for Memory Management and PYTORCH_CUDA_ALLOC_CONF

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/var/lib/jenkins/workspace/benchmarks/dynamo/common.py", line 3194, in run
    ) = runner.load_model(
  File "/var/lib/jenkins/workspace/benchmarks/dynamo/torchbench.py", line 385, in load_model
    self.validate_model(model, example_inputs)
  File "/var/lib/jenkins/workspace/benchmarks/dynamo/common.py", line 1750, in validate_model
    raise NotImplementedError("Eager model failed to run") from e
NotImplementedError: Eager model failed to run

@atalman
Copy link
Contributor

atalman commented Jul 17, 2023

@pytorchbot revert -c ignoredsignal -m "Failing cuda11.8-py3.10-gcc7-sm86 / test (inductor_torchbench_dynamic) with CUDA oom"

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@voznesenskym your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Jul 17, 2023
This reverts commit 82fb5ed.

Reverted #103205 on behalf of https://github.com/atalman due to Failing cuda11.8-py3.10-gcc7-sm86 / test (inductor_torchbench_dynamic) with CUDA oom ([comment](#103205 (comment)))
@voznesenskym
Copy link
Collaborator Author

I wonder if we are failing because we graph break less now?

@voznesenskym
Copy link
Collaborator Author

I looked at the log, the oom is on another model, it seems?

2023-07-15T16:57:45.0635699Z cuda eval  llama                              
2023-07-15T16:58:22.4798235Z skipping cudagraphs due to input mutation
2023-07-15T16:58:22.8327903Z [2023-07-15 16:58:22,832] torch._dynamo.utils: [ERROR] Accuracy failed: allclose not within tol=0.0001
2023-07-15T16:58:22.8344321Z fail_accuracy

is the accuracy failure.

@voznesenskym voznesenskym reopened this Jul 17, 2023
voznesenskym added a commit that referenced this pull request Jul 17, 2023
This reverts commit 94b3f9f.

Fix

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Jul 17, 2023
This reverts commit 94b3f9f.

Fix

ghstack-source-id: 378a74aaaaafb0e468b55ad72d2cd9a9fea1a9c7
Pull Request resolved: #105345
@albanD albanD removed their request for review July 17, 2023 19:24
voznesenskym added a commit that referenced this pull request Jul 17, 2023
…r improved graph breaks"

This reverts commit 94b3f9f.

Fix

cc penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78 aakhundov

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Jul 17, 2023
This reverts commit 94b3f9f.

Fix

ghstack-source-id: c4d5ce1e2d64ac905d46aff0396b8349271aa49f
Pull Request resolved: #105345

Fix
pytorchmergebot pushed a commit that referenced this pull request Jul 17, 2023
…graph breaks (#105345)

This reverts commit 94b3f9f.

Fix

Pull Request resolved: #105345
Approved by: https://github.com/atalman
voznesenskym added a commit that referenced this pull request Jul 19, 2023
Set initial

ghstack-source-id: aee0c9f4627d63000ba146006d494a2dbf7adcbf
Pull Request resolved: #103205

wip

WIP

WIP

WIP

lint

lint

lint

lint

lint

Fix

Fix

fixup

fixup

Fix
voznesenskym added a commit that referenced this pull request Jul 21, 2023
Set initial

ghstack-source-id: aee0c9f4627d63000ba146006d494a2dbf7adcbf
Pull Request resolved: #103205

wip

WIP

WIP

WIP

lint

lint

lint

lint

lint

Fix

Fix

fixup

fixup

Fix
voznesenskym added a commit that referenced this pull request Aug 7, 2023
Set initial

ghstack-source-id: aee0c9f4627d63000ba146006d494a2dbf7adcbf
Pull Request resolved: #103205

wip

WIP

WIP

WIP

lint

lint

lint

lint

lint

Fix

Fix

fixup

fixup

Fix
@facebook-github-bot facebook-github-bot deleted the gh/voznesenskym/147/head branch August 11, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants