-
Couldn't load subscription status.
- Fork 87
[torchlib] Consolidate all overloads and prevent new ones from being created #2621
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR consolidates overloaded functions in the torch_lib by removing the concept of private functions and preventing new overloads from being created for the same operation name. The changes focus on simplifying the registration system and merging boolean indexing operations with their regular counterparts.
- Removes the
privateparameter and functionality from the registration system - Consolidates boolean and regular index operations into unified functions
- Adds validation to prevent duplicate overload registrations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| onnxscript/function_libs/torch_lib/registration.py | Removes private function support and adds overload duplication prevention |
| onnxscript/function_libs/torch_lib/ops/core.py | Consolidates index and index_put functions to handle both boolean and integer indexing |
| tests/function_libs/torch_lib/ops_test_data.py | Removes separate boolean index test entries and duplicates |
| onnxscript/function_libs/torch_lib/ops/nn.py | Removes outdated comment about private functions |
| onnxscript/function_libs/tools/torch_lib/deduce_type_constraints_test.py | Updates to exclude removed private functions from iteration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2621 +/- ##
==========================================
- Coverage 70.39% 70.38% -0.01%
==========================================
Files 222 222
Lines 26275 26287 +12
Branches 2629 2637 +8
==========================================
+ Hits 18496 18503 +7
- Misses 6859 6862 +3
- Partials 920 922 +2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
This PR implements #2580 by combining all overloads in torchlib and remove the ability to register new ones. It is done in a BC compatible fashion and should work with released versions of PyTorch.
From now on all logic for a single aten OpOverload should be implemented by a single torchlib function to ensure 1-to-1 mapping.