-
Notifications
You must be signed in to change notification settings - Fork 526
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
pt: add 4 tabulate_fusion op #3877
Conversation
…o devel pt_tabulate_fusion recommit by Yan.
WalkthroughWalkthroughThis update introduces a new file, Changes
Sequence Diagram(s)sequenceDiagram
participant Developer as Developer
participant BuildSys as Build System
participant PyTorch as PyTorch Library
participant Model as Deep Learning Model
Developer->>BuildSys: Modify CMakeLists.txt
BuildSys->>tabulate_multi_device.cc: Add to OP_SRC
Note over tabulate_multi_device.cc: New functions and classes added for operations
Model->>tabulate_multi_device.cc: Request fusion operation A (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeAOp
PyTorch->>Model: Perform fusion operation A
Model->>tabulate_multi_device.cc: Request gradient of fusion operation A (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeAGradOp
PyTorch->>Model: Compute gradient of fusion operation A
Model->>tabulate_multi_device.cc: Request gradient of gradient of fusion operation A (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeAGradGradOp
PyTorch->>Model: Perform gradient of gradient computation
Model->>tabulate_multi_device.cc: Request fusion operation T (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeTOp
PyTorch->>Model: Perform fusion operation T
Model->>tabulate_multi_device.cc: Request fusion operation R (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeROp
PyTorch->>Model: Perform fusion operation R
Model->>tabulate_multi_device.cc: Request attention-based fusion operation (forward)
tabulate_multi_device.cc->>PyTorch: Register TabulateFusionSeAttenOp
PyTorch->>Model: Perform attention-based fusion operation
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files not summarized due to errors (1)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #3877 +/- ##
==========================================
- Coverage 82.74% 82.72% -0.02%
==========================================
Files 518 518
Lines 50215 50414 +199
Branches 2982 3018 +36
==========================================
+ Hits 41548 41705 +157
- Misses 7757 7773 +16
- Partials 910 936 +26 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 7
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 1
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.
Actionable comments posted: 3
TORCH_LIBRARY_FRAGMENT(deepmd, m) { | ||
m.def("tabulate_fusion_se_t", tabulate_fusion_se_t); | ||
} | ||
TORCH_LIBRARY_FRAGMENT(deepmd, m) { |
Check notice
Code scanning / CodeQL
Unused static function Note
TORCH_LIBRARY_FRAGMENT_static_init_deepmd_5
TORCH_LIBRARY_FRAGMENT(deepmd, m) { | ||
m.def("tabulate_fusion_se_atten", tabulate_fusion_se_atten); | ||
} | ||
TORCH_LIBRARY_FRAGMENT(deepmd, m) { |
Check notice
Code scanning / CodeQL
Unused static function Note
TORCH_LIBRARY_FRAGMENT_static_init_deepmd_4
TORCH_LIBRARY_FRAGMENT(deepmd, m) { | ||
m.def("tabulate_fusion_se_a", tabulate_fusion_se_a); | ||
} | ||
TORCH_LIBRARY_FRAGMENT(deepmd, m) { |
Check notice
Code scanning / CodeQL
Unused static function Note
TORCH_LIBRARY_FRAGMENT_static_init_deepmd_3
last_layer_size); | ||
} | ||
|
||
TORCH_LIBRARY_FRAGMENT(deepmd, m) { |
Check notice
Code scanning / CodeQL
Unused static function Note
TORCH_LIBRARY_FRAGMENT_static_init_deepmd_2
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.
Actionable comments posted: 10
Outside diff range and nitpick comments (1)
source/op/pt/tabulate_multi_device.cc (1)
440-476
: Consider adding detailed logging for debugging purposes inTabulateFusionSeRGradGradForward
, which could help trace computation values step-by-step.
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 5
Outside diff range and nitpick comments (3)
source/tests/pt/test_tabulate_fusion_se_r.py (2)
1-1
: Add a module docstring to describe the purpose and functionality of this test module.
11-1100
: Ensure proper documentation for the test class and its methods to improve maintainability and understandability.source/tests/pt/test_tabulate_fusion_se_a.py (1)
1072-1073
: Consider adding comments describing the purpose and contents of these tensors to improve code readability and maintainability.Also applies to: 1076-1076, 1079-1098, 1099-1167
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 9
Outside diff range and nitpick comments (3)
source/op/pt/tabulate_multi_device.cc (3)
21-71
: Ensure that the input validation messages in theTabulateFusionSeAForward
function are clear and precise to help users understand the expected input dimensions.
73-121
: InTabulateFusionSeAGradForward
, consider adding detailed logging or comments explaining the gradient computation logic for better maintainability.
182-222
: The functionTabulateFusionSeTForward
performs tensor computations but lacks inline comments explaining the steps. Adding comments would enhance understandability.
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 12
Outside diff range and nitpick comments (2)
source/tests/pt/test_tabulate_fusion_se_a.py (2)
1413-1413
: Consider adding a docstring to thetest_forward
method to describe its purpose and the specific conditions it tests.
Adding a docstring would improve the maintainability of the test suite by making it easier for other developers to understand the purpose and functionality of each test case.
1436-1436
: Consider adding a docstring to thetest_backward
method to describe its purpose and the specific conditions it tests.
Adding a docstring would enhance the readability and maintainability of the test suite, providing clarity on what each test case is designed to verify.
@coderabbitai resolve |
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
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.
Actionable comments posted: 3
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
**OP**: tabulate_multi_device.cc -> tabulate_fusion_se_a & atten & r & t **Compile**: add compile file **Test**: source/tests, se_t has no test, and all other ops pass the forward and backward tests. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced support for tabulating fusion operations for deep learning models using PyTorch tensors. - Added functions for forward, backward, and gradient computations for various fusion operations, handling different data types and device types. - Registered new operations with PyTorch libraries for seamless integration. - **Chores** - Updated build configuration to include new source files for fusion operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
OP: tabulate_multi_device.cc -> tabulate_fusion_se_a & atten & r & t
Compile: add compile file
Test: source/tests, se_t has no test, and all other ops pass the forward and backward tests.
Summary by CodeRabbit
New Features
Chores