modify the conditions as PythonModuleVariable #116856

jiayisunx · 2024-01-05T10:42:28Z

Motivation

The current code of value in [torch.backends.cudnn, torch.ops] requires value to have the implementation of __eq__. If the value is a custom object and does not implement __eq__, dynamo will throw error. For example, ConvolutionOpContext, the custom 'torch._C.ScriptClass' object registered in IPEX, dynamo will throw the following error:

torch._dynamo.exc.InternalTorchDynamoError: 'eq' is not implemented for torch.torch.classes.ipex_prepack.ConvolutionOpContext

I think this is a common issue, To avoid this issue, the PR replaces the current code value in [torch.backends.cudnn, torch.ops]with isinstance(value, (torch.backends.cudnn.CudnnModule, torch._ops._Ops)

Stack from ghstack (oldest at bottom):

-> modify the conditions as PythonModuleVariable #116856

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

pytorch-bot · 2024-01-05T10:42:31Z

🔗 Helpful Links

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

📄 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! (2 Unrelated Failures)

As of commit 706470a with merge base 5d2d21a ():

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

inductor / rocm5.7-py3.8-inductor / test (inductor, 1, 1, linux.rocm.gpu.2) (gh)
Process completed with exit code 1.

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

trunk / linux-focal-rocm5.7-py3.8 / test (default, 1, 1, linux.rocm.gpu, unstable) (gh)
Process completed with exit code 1.

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

ghstack-source-id: adc78fd79d412f21bbb685a0915f3c053cf7f40f Pull Request resolved: #116856

[ghstack-poisoned]

jiayisunx · 2024-01-08T01:27:21Z

@pytorchbot rebase

pytorchmergebot · 2024-01-08T01:29:01Z

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

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

pytorchmergebot · 2024-01-08T01:29:14Z

Successfully rebased gh/jiayisunx/1/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/116856)

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

ghstack-source-id: 62ae6e8578689722273a4fca815e0ff23feb0383 Pull Request resolved: #116856

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

Skylion007 · 2024-01-09T16:24:39Z

torch/_dynamo/variables/builder.py

@@ -695,8 +695,14 @@ def build_key_value(k, v):
            istype(value, (types.ModuleType, replay_record.DummyModule))
            # type(torch.backends.cudnn) -> <class 'torch.backends.cudnn.CudnnModule'>
            # type(torch.ops) -> <class 'torch._ops._Ops'>
-            or value in [torch.backends.cudnn, torch.ops]


If this isn't working, wouldn't:

Suggested change

or value in [torch.backends.cudnn, torch.ops]

or any(value is obj for obj in (torch.backends.cudnn, torch.ops))

work?

It works, thanks!

jansel

This seems like a semantic change. isinstance is not the same as ==.

I'd suggest replacing it with is instread as @Skylion007 suggests.

ghstack-source-id: c0f598322c07b3aa1778add499b3f9c02b8f6d58 Pull Request resolved: #116856

## Motivation The current code of `value in [torch.backends.cudnn, torch.ops]` requires `value` to have the implementation of `__eq__`. If the value is a custom object and does not implement `__eq__`, dynamo will throw error. For example, ConvolutionOpContext, the custom 'torch._C.ScriptClass' object registered in IPEX, dynamo will throw the following error: **torch._dynamo.exc.InternalTorchDynamoError: '__eq__' is not implemented for __torch__.torch.classes.ipex_prepack.ConvolutionOpContext** I think this is a common issue, To avoid this issue, the PR replaces the current code `value in [torch.backends.cudnn, torch.ops]`with `isinstance(value, (torch.backends.cudnn.CudnnModule, torch._ops._Ops)` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

pytorchmergebot · 2024-01-11T00:56:00Z

Merge failed

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

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

## Motivation The current code of `value in [torch.backends.cudnn, torch.ops]` requires `value` to have the implementation of `__eq__`. If the value is a custom object and does not implement `__eq__`, dynamo will throw error. For example, ConvolutionOpContext, the custom 'torch._C.ScriptClass' object registered in IPEX, dynamo will throw the following error: **torch._dynamo.exc.InternalTorchDynamoError: '__eq__' is not implemented for __torch__.torch.classes.ipex_prepack.ConvolutionOpContext** I think this is a common issue, To avoid this issue, the PR replaces the current code `value in [torch.backends.cudnn, torch.ops]`with `isinstance(value, (torch.backends.cudnn.CudnnModule, torch._ops._Ops)` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

ghstack-source-id: b04242dac61383025f6e59705ce74e64db71e09b Pull Request resolved: #116856

## Motivation The current code of `value in [torch.backends.cudnn, torch.ops]` requires `value` to have the implementation of `__eq__`. If the value is a custom object and does not implement `__eq__`, dynamo will throw error. For example, ConvolutionOpContext, the custom 'torch._C.ScriptClass' object registered in IPEX, dynamo will throw the following error: **torch._dynamo.exc.InternalTorchDynamoError: '__eq__' is not implemented for __torch__.torch.classes.ipex_prepack.ConvolutionOpContext** I think this is a common issue, To avoid this issue, the PR replaces the current code `value in [torch.backends.cudnn, torch.ops]`with `isinstance(value, (torch.backends.cudnn.CudnnModule, torch._ops._Ops)` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

ghstack-source-id: 8558ef9d9be2ce20e63ef705f608b88ac1be202b Pull Request resolved: #116856

jiayisunx · 2024-01-15T11:07:54Z

@pytorchbot merge

pytorchmergebot · 2024-01-15T11:10:10Z

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

github-actions bot added module: dynamo ciflow/inductor labels Jan 5, 2024

jiayisunx added a commit that referenced this pull request Jan 5, 2024

modify the conditions as PythonModuleVariable

7db69a4

ghstack-source-id: adc78fd79d412f21bbb685a0915f3c053cf7f40f Pull Request resolved: #116856

modify the conditions as PythonModuleVariable

c032695

[ghstack-poisoned]

pytorchbot added the open source label Jan 5, 2024

Update on "modify the conditions as PythonModuleVariable"

f00fd89

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

jiayisunx marked this pull request as draft January 8, 2024 02:36

jiayisunx added 2 commits January 8, 2024 16:04

Update on "modify the conditions as PythonModuleVariable"

2b61e6f

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

Update on "modify the conditions as PythonModuleVariable"

060a1c9

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

Update on "modify the conditions as PythonModuleVariable"

0356a52

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

jiayisunx added a commit that referenced this pull request Jan 9, 2024

modify the conditions as PythonModuleVariable

e079ad2

ghstack-source-id: 62ae6e8578689722273a4fca815e0ff23feb0383 Pull Request resolved: #116856

Update on "modify the conditions as PythonModuleVariable"

488bb5e

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv chenyang78 aakhundov kadeng [ghstack-poisoned]

jiayisunx added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 9, 2024

jiayisunx requested review from jansel and jgong5 January 9, 2024 07:50

jiayisunx marked this pull request as ready for review January 9, 2024 07:50

Skylion007 reviewed Jan 9, 2024

View reviewed changes

jansel requested changes Jan 9, 2024

View reviewed changes

jiayisunx added a commit that referenced this pull request Jan 10, 2024

modify the conditions as PythonModuleVariable

4cadc47

ghstack-source-id: c0f598322c07b3aa1778add499b3f9c02b8f6d58 Pull Request resolved: #116856

jiayisunx requested review from Skylion007 and jansel January 10, 2024 08:19

pytorchmergebot removed the merging label Jan 11, 2024

jiayisunx added 2 commits January 12, 2024 18:44

jiayisunx added a commit that referenced this pull request Jan 12, 2024

modify the conditions as PythonModuleVariable

b6c7ba0

ghstack-source-id: b04242dac61383025f6e59705ce74e64db71e09b Pull Request resolved: #116856

jiayisunx added 2 commits January 15, 2024 13:33

jiayisunx added a commit that referenced this pull request Jan 15, 2024

modify the conditions as PythonModuleVariable

bedc17b

ghstack-source-id: 8558ef9d9be2ce20e63ef705f608b88ac1be202b Pull Request resolved: #116856

pytorchmergebot added the merging label Jan 15, 2024

pytorchmergebot closed this in d9b265a Jan 15, 2024

pytorchmergebot added Merged and removed merging labels Jan 15, 2024

facebook-github-bot deleted the gh/jiayisunx/1/head branch January 18, 2024 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modify the conditions as PythonModuleVariable #116856

modify the conditions as PythonModuleVariable #116856

jiayisunx commented Jan 5, 2024 •

edited

Loading

pytorch-bot bot commented Jan 5, 2024 •

edited

Loading

jiayisunx commented Jan 8, 2024

pytorchmergebot commented Jan 8, 2024

pytorchmergebot commented Jan 8, 2024

Skylion007 Jan 9, 2024

jiayisunx Jan 10, 2024

jansel left a comment

pytorchmergebot commented Jan 11, 2024

jiayisunx commented Jan 15, 2024

pytorchmergebot commented Jan 15, 2024

	or value in [torch.backends.cudnn, torch.ops]
	or any(value is obj for obj in (torch.backends.cudnn, torch.ops))

modify the conditions as PythonModuleVariable #116856

modify the conditions as PythonModuleVariable #116856

Conversation

jiayisunx commented Jan 5, 2024 • edited Loading

Motivation

pytorch-bot bot commented Jan 5, 2024 • edited Loading

🔗 Helpful Links

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

✅ You can merge normally! (2 Unrelated Failures)

jiayisunx commented Jan 8, 2024

pytorchmergebot commented Jan 8, 2024

pytorchmergebot commented Jan 8, 2024

Skylion007 Jan 9, 2024

Choose a reason for hiding this comment

jiayisunx Jan 10, 2024

Choose a reason for hiding this comment

jansel left a comment

Choose a reason for hiding this comment

pytorchmergebot commented Jan 11, 2024

Merge failed

jiayisunx commented Jan 15, 2024

pytorchmergebot commented Jan 15, 2024

Merge started

jiayisunx commented Jan 5, 2024 •

edited

Loading

pytorch-bot bot commented Jan 5, 2024 •

edited

Loading