[dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

peterbell10 · 2023-10-28T14:52:48Z

Stack from ghstack (oldest at bottom):

-> [dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

Currently custom VariableTrackers exist for classes that live outside of pytorch.
For these cases dynamo currently eagerly imports the module to get the class
object to compare against.

This instead uses sys.modules.get("module.path") such that the module is never
imported by dynamo itself, but if the user has imported the module then we will
still access the module and grab the type we need to compare against.

I noticed this issue because importing KeyedJaggedTensor fails half-way
through if fbgemm_gpu has been built with an incompatible PyTorch version, in
which case it retries the import again each time!

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng

pytorch-bot · 2023-10-28T14:52:51Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit e2d6ecd with merge base 493b52b ():

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

pull / linux-focal-py3_8-clang9-xla / test (xla, 1, 1, linux.12xlarge) (gh)

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

Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the Class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! ghstack-source-id: fe3b46025be307284124bf252ff12c66ee2b7021 Pull Request resolved: #112319

Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the Class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! [ghstack-poisoned]

Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the Class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! ghstack-source-id: c660171c51a9207d92055a4634feef0a61805445 Pull Request resolved: #112319

…leTrackers" Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]

Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the Class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! ghstack-source-id: 01a33edff568568043f3a3bf2d5f655dd625e2fc Pull Request resolved: #112319

…leTrackers" Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]

lezcano

LGTM as in "looks great to me" but we can wait for @voznesenskym see if he has any comments.

lezcano · 2023-10-31T09:45:26Z

torch/_dynamo/variables/torch.py

@@ -132,23 +132,6 @@ def remap_as_fn___rand__(*args):
 }


-try:


side effects at import time: 0/10

peterbell10 · 2023-11-03T18:53:20Z

@voznesenskym ping

ezyang

Very nice, thanks

peterbell10 · 2023-11-07T18:01:34Z

@pytorchbot merge -r

pytorchmergebot · 2023-11-07T18:03:24Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2023-11-07T18:03:28Z

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/viable/strict gh/peterbell10/642/orig returned non-zero exit code 1

Rebasing (1/1)
Auto-merging torch/_dynamo/variables/dicts.py
CONFLICT (content): Merge conflict in torch/_dynamo/variables/dicts.py
Auto-merging torch/_dynamo/variables/torch.py
CONFLICT (content): Merge conflict in torch/_dynamo/variables/torch.py
Auto-merging torch/_dynamo/variables/user_defined.py
error: could not apply 4f3740bbd32... [dynamo] Avoid eager imports of classes with custom VariableTrackers
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 4f3740bbd32... [dynamo] Avoid eager imports of classes with custom VariableTrackers

Raised by https://github.com/pytorch/pytorch/actions/runs/6788481424

Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the Class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! ghstack-source-id: a77b1ca4d6259b7b89115737f79d1507ed9336c5 Pull Request resolved: #112319

…leTrackers" Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng [ghstack-poisoned]

peterbell10 · 2023-11-07T19:24:07Z

@pytorchbot merge

pytorchmergebot · 2023-11-07T19:25:55Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

ezyang · 2023-11-07T20:01:10Z

@pytorchbot merge

pytorchmergebot · 2023-11-07T20:03:02Z

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 · 2023-11-07T21:54:32Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / linux-focal-py3_8-clang9-xla / test (xla, 1, 1, linux.12xlarge)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

lezcano · 2023-11-07T22:27:45Z

@pytorchbot merge

pytorchmergebot · 2023-11-07T22:29:50Z

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

…ytorch#112319) Currently custom VariableTrackers exist for classes that live outside of pytorch. For these cases dynamo currently eagerly imports the module to get the class object to compare against. This instead uses `sys.modules.get("module.path")` such that the module is never imported by dynamo itself, but if the user has imported the module then we will still access the module and grab the type we need to compare against. I noticed this issue because importing `KeyedJaggedTensor` fails half-way through if `fbgemm_gpu` has been built with an incompatible PyTorch version, in which case it retries the import again each time! Pull Request resolved: pytorch#112319 Approved by: https://github.com/lezcano, https://github.com/ezyang

github-actions bot added module: dynamo ciflow/inductor labels Oct 28, 2023

pytorchbot added the open source label Oct 28, 2023

peterbell10 requested a review from voznesenskym October 28, 2023 21:01

peterbell10 marked this pull request as ready for review October 28, 2023 21:01

peterbell10 requested a review from lezcano October 30, 2023 19:45

lezcano approved these changes Oct 31, 2023

View reviewed changes

peterbell10 requested a review from ezyang November 7, 2023 17:50

ezyang approved these changes Nov 7, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 7, 2023

pytorchmergebot added the merging label Nov 7, 2023

pytorchmergebot removed the merging label Nov 7, 2023

ezyang added topic: bug fixes topic category release notes: dynamo labels Nov 7, 2023

pytorchmergebot added the merging label Nov 7, 2023

pytorchmergebot removed the merging label Nov 7, 2023

pytorchmergebot added the merging label Nov 7, 2023

pytorchmergebot added Merged and removed merging labels Nov 7, 2023

pytorchmergebot closed this in a3a2486 Nov 7, 2023

facebook-github-bot deleted the gh/peterbell10/642/head branch November 11, 2023 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

[dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

peterbell10 commented Oct 28, 2023 •

edited

pytorch-bot bot commented Oct 28, 2023 •

edited

lezcano left a comment

lezcano Oct 31, 2023

peterbell10 commented Nov 3, 2023 •

edited

ezyang left a comment

peterbell10 commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

peterbell10 commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

ezyang commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

lezcano commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

[dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

[dynamo] Avoid eager imports of classes with custom VariableTrackers #112319

Conversation

peterbell10 commented Oct 28, 2023 • edited

pytorch-bot bot commented Oct 28, 2023 • edited

🔗 Helpful Links

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

✅ You can merge normally! (1 Unrelated Failure)

lezcano left a comment

Choose a reason for hiding this comment

lezcano Oct 31, 2023

Choose a reason for hiding this comment

peterbell10 commented Nov 3, 2023 • edited

ezyang left a comment

Choose a reason for hiding this comment

peterbell10 commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

peterbell10 commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

Merge failed

ezyang commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

Merge started

pytorchmergebot commented Nov 7, 2023

Merge failed

lezcano commented Nov 7, 2023

pytorchmergebot commented Nov 7, 2023

Merge started

peterbell10 commented Oct 28, 2023 •

edited

pytorch-bot bot commented Oct 28, 2023 •

edited

peterbell10 commented Nov 3, 2023 •

edited