Skip to content

Compute type_equal() without reference to backend()#53823

Closed
ezyang wants to merge 7 commits intogh/ezyang/943/basefrom
gh/ezyang/943/head
Closed

Compute type_equal() without reference to backend()#53823
ezyang wants to merge 7 commits intogh/ezyang/943/basefrom
gh/ezyang/943/head

Conversation

@ezyang
Copy link
Copy Markdown
Contributor

@ezyang ezyang commented Mar 11, 2021

Stack from ghstack:

Argument for correctness: type_equal previous compared if backends
are equal. Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective. Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang ezyang@fb.com

Differential Revision: D27036575

Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Mar 11, 2021

💊 CI failures summary and remediations

As of commit 2b53b5f (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

ezyang added a commit that referenced this pull request Mar 11, 2021
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ghstack-source-id: e16f265
Pull Request resolved: #53823
@ezyang ezyang requested review from bhosmer and smessmer March 11, 2021 16:35
Copy link
Copy Markdown

@bhosmer bhosmer left a comment

Choose a reason for hiding this comment

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

This makes sense but it seems pretty fragile.

  • any idea what it saves us, perf-wise?
  • can we throw in a lightweight injectivity test somewhere?

@ezyang
Copy link
Copy Markdown
Contributor Author

ezyang commented Mar 12, 2021

any idea what it saves us, perf-wise?

My intention wasn't really perf; I just really didn't want to put Meta in the list of Backends XD

can we throw in a lightweight injectivity test somewhere?

I... guess? But honestly I'd rather spend the time just getting rid of Backend (as we have kept saying we would do, and then haven't)

@bhosmer
Copy link
Copy Markdown

bhosmer commented Mar 13, 2021

I... guess? But honestly I'd rather spend the time just getting rid of Backend (as we have kept saying we would do, and then haven't)

Makes sense. So maybe just a comment on computeDispatchKey() noting that we use its output as a proxy for backend so it must only return keys for which dispatchKeyToBackend() is injective

Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
ezyang added 3 commits March 14, 2021 08:30
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Differential Revision: [D27036575](https://our.internmc.facebook.com/intern/diff/D27036575)

[ghstack-poisoned]
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Differential Revision: [D27036575](https://our.internmc.facebook.com/intern/diff/D27036575)

[ghstack-poisoned]
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Differential Revision: [D27036575](https://our.internmc.facebook.com/intern/diff/D27036575)

[ghstack-poisoned]
ezyang added 2 commits March 15, 2021 11:23
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Differential Revision: [D27036575](https://our.internmc.facebook.com/intern/diff/D27036575)

[ghstack-poisoned]
Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Differential Revision: [D27036575](https://our.internmc.facebook.com/intern/diff/D27036575)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ezyang merged this pull request in d47fd3d.

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/943/head branch March 20, 2021 14:15
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Pull Request resolved: pytorch#53823

Argument for correctness: type_equal previous compared if backends
are equal.  Backend is computed by translation from dispatch key.
I verified that computeDispatchKey never computed a weird
dispatch key (e.g., AutogradXLA), so that dispatchKeyToBackend
was effectively injective.  Then it is always valid to compare
the arguments of an injective function for equality, rather than
the output of the injective function.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan: Imported from OSS

Reviewed By: mruberry

Differential Revision: D27036575

Pulled By: ezyang

fbshipit-source-id: 6aeafc89f287da0bc0065bd21c1adb5e272dbb81
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.

4 participants