Verify types in custom op schemas #124520

zou3519 · 2024-04-19T20:36:54Z

Stack from ghstack (oldest at bottom):

-> Verify types in custom op schemas #124520

Before this PR, we didn't check that types in a schema were valid. This
is because TorchScript treats unknown types as type variables.

This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this,
we add an allow_typevars flag to parseSchema so that TorchScript can
use allow_typevars=True. We also add some error messages for common
mistakes (e.g. using int64_t or double in schema).

Test Plan:

new tests

Differential Revision: D56432690

cc @ezyang @gchanan

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests [ghstack-poisoned]

pytorch-bot · 2024-04-19T20:36:57Z

🔗 Helpful Links

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

📄 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 515529d with merge base 2f53747 ():

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

pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 1, 5, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
inductor/test_unbacked_symints.py::TestUnbackedSymintsCUDA::test_vertical_pointwise_reduction_fusion_cuda

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

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests [ghstack-poisoned]

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests ghstack-source-id: 04eac8a Pull Request resolved: #124520

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests [ghstack-poisoned]

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests ghstack-source-id: 386c2dc Pull Request resolved: #124520

zou3519 · 2024-04-22T19:17:19Z

@zou3519 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

zou3519 · 2024-04-22T21:24:22Z

This is technically BC-breaking. If you want the old behavior, use torch::schema(<schema_str>, allow_typevars=True) when defining an operator.

williamwen42

lgtm - will defer due to BC breaking change.

albanD

This doesn't change TORCH_LIBRARY so I'm good :)

zou3519 · 2024-04-23T14:16:23Z

@pytorchbot merge

pytorchmergebot · 2024-04-23T14:18:19Z

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 · 2024-04-26T08:42:15Z

@zou3519 your PR has been successfully reverted.

This reverts commit 1418887. Reverted #124520 on behalf of https://github.com/jeanschmidt due to Breaking internal tests check D56588015 for more details ([comment](#124520 (comment)))

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests Differential Revision: [D56432690](https://our.internmc.facebook.com/intern/diff/D56432690) Pull Request resolved: pytorch#124520 Approved by: https://github.com/albanD

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests Differential Revision: [D56432690](https://our.internmc.facebook.com/intern/diff/D56432690) Pull Request resolved: #124520 Approved by: https://github.com/albanD

This reverts commit 5b98d43. Reverted #124520 on behalf of https://github.com/zou3519 due to broke static runtime tests ([comment](#124520 (comment)))

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests Differential Revision: [D56432690](https://our.internmc.facebook.com/intern/diff/D56432690) Pull Request resolved: pytorch#124520 Approved by: https://github.com/albanD

This reverts commit 1418887. Reverted pytorch#124520 on behalf of https://github.com/jeanschmidt due to Breaking internal tests check D56588015 for more details ([comment](pytorch#124520 (comment)))

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests Differential Revision: [D56432690](https://our.internmc.facebook.com/intern/diff/D56432690) cc ezyang gchanan [ghstack-poisoned]

Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: - new tests Differential Revision: [D56432690](https://our.internmc.facebook.com/intern/diff/D56432690) Pull Request resolved: #124520 Approved by: https://github.com/albanD ghstack-source-id: 1232820

Summary: co-dev reland of pytorch/pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Differential Revision: D57666659

Summary: X-link: pytorch/executorch#3705 co-dev reland of pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: Wait for tests Differential Revision: D57666659

Summary: X-link: pytorch/pytorch#126861 co-dev reland of pytorch/pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Differential Revision: D57666659

Summary: X-link: pytorch/pytorch#126861 co-dev reland of pytorch/pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Reviewed By: albanD Differential Revision: D57666659

Summary: X-link: pytorch/executorch#3705 co-dev reland of pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: Wait for tests Reviewed By: albanD Differential Revision: D57666659

Summary: X-link: pytorch/pytorch#126861 Pull Request resolved: #3705 co-dev reland of pytorch/pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Reviewed By: albanD Differential Revision: D57666659 fbshipit-source-id: 25b0c247229ea78966725a3678dd01fdcc02c91d

Summary: co-dev reland of #124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: Wait for tests Differential Revision: D57666659 Pull Request resolved: #126861 Approved by: https://github.com/albanD

zou3519 · 2024-05-24T01:05:59Z

landed in #126861

Summary: X-link: pytorch/pytorch#126861 Pull Request resolved: pytorch#3705 co-dev reland of pytorch/pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Reviewed By: albanD Differential Revision: D57666659 fbshipit-source-id: 25b0c247229ea78966725a3678dd01fdcc02c91d

Summary: co-dev reland of pytorch#124520, which requires the removal of some executorch tests. Before this PR, we didn't check that types in a schema were valid. This is because TorchScript treats unknown types as type variables. This PR checks types in a schema for the TORCH_LIBRARY APIs. To do this, we add an `allow_typevars` flag to parseSchema so that TorchScript can use allow_typevars=True. We also add some error messages for common mistakes (e.g. using int64_t or double in schema). Test Plan: Wait for tests Differential Revision: D57666659 Pull Request resolved: pytorch#126861 Approved by: https://github.com/albanD

zou3519 requested a review from mikaylagawarecki as a code owner April 19, 2024 20:36

This was referenced Apr 19, 2024

[opcheck] Stop doing test_aot_dispatch_static by default #124483

Closed

Make some kernel static asserts clearer #124519

Closed

pytorch-bot bot added ci-td-distributed release notes: jit release notes category labels Apr 19, 2024

zou3519 added ciflow/trunk Trigger trunk jobs on your pull request keep-going Don't stop on first failure, keep running tests until the end labels Apr 19, 2024

mikaylagawarecki removed their request for review April 19, 2024 21:16

zou3519 added release notes: composability release notes category module: bc-breaking Related to a BC-breaking change and removed release notes: jit release notes category labels Apr 22, 2024

pytorch-bot bot added the topic: bc_breaking label Apr 22, 2024

zou3519 requested review from ezyang, albanD and williamwen42 April 22, 2024 21:24

williamwen42 reviewed Apr 22, 2024

View reviewed changes

albanD approved these changes Apr 22, 2024

View reviewed changes

pytorchmergebot added the merging label Apr 23, 2024

pytorchmergebot reopened this Apr 26, 2024

pytorch-bot bot pushed a commit that referenced this pull request May 3, 2024

Revert "Verify types in custom op schemas (#124520)"

f9ba66d

This reverts commit 5b98d43. Reverted #124520 on behalf of https://github.com/zou3519 due to broke static runtime tests ([comment](#124520 (comment)))

malfet added topic: bc breaking topic category and removed topic: bc_breaking labels May 20, 2024

This was referenced May 22, 2024

Verify types in custom op schemas pytorch/executorch#3705

Closed

[Reland] Verify types in custom op schemas #126861

Closed

zou3519 closed this May 24, 2024

github-actions bot deleted the gh/zou3519/979/head branch June 23, 2024 01:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Verify types in custom op schemas #124520

Verify types in custom op schemas #124520

Uh oh!

zou3519 commented Apr 19, 2024 •

edited by pytorch-bot bot

Loading

Uh oh!

pytorch-bot bot commented Apr 19, 2024 •

edited

Loading

Uh oh!

zou3519 commented Apr 22, 2024

Uh oh!

zou3519 commented Apr 22, 2024

Uh oh!

williamwen42 left a comment

Uh oh!

albanD left a comment

Uh oh!

zou3519 commented Apr 23, 2024

Uh oh!

pytorchmergebot commented Apr 23, 2024

Uh oh!

pytorchmergebot commented Apr 26, 2024

Uh oh!

zou3519 commented May 24, 2024

Uh oh!

Uh oh!

Verify types in custom op schemas #124520

Verify types in custom op schemas #124520

Uh oh!

Conversation

zou3519 commented Apr 19, 2024 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

zou3519 commented Apr 22, 2024

Uh oh!

zou3519 commented Apr 22, 2024

Uh oh!

williamwen42 left a comment

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

zou3519 commented Apr 23, 2024

Uh oh!

pytorchmergebot commented Apr 23, 2024

Merge started

Uh oh!

pytorchmergebot commented Apr 26, 2024

Uh oh!

zou3519 commented May 24, 2024

Uh oh!

Uh oh!

zou3519 commented Apr 19, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Apr 19, 2024 •

edited

Loading