-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Fix] Fix the wrong check for tuple node in #18163 #18170
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
Conversation
|
We noticed that one CI job has been running for a very long time—17 hours since yesterday. Is this normal? Looks like it's stuck on the last test, or does this job usually take this long? [2025-07-29T10:25:53.581Z] PASSED
[2025-07-29T10:25:53.581Z] tests/python/relax/test_transform_few_shot_tuning.py::test_funcs[False-MatMul] 2025-07-29 10:25:53 [INFO] LocalBuilder: max_workers = 16
[2025-07-29T10:25:57.744Z] PASSED
[2025-07-29T10:25:57.745Z] tests/python/relax/test_transform_few_shot_tuning.py::test_funcs[False-Fuse_Mean_Cast1] 2025-07-29 10:25:57 [INFO] LocalBuilder: max_workers = 16
[2025-07-29T10:26:01.010Z] PASSED
[2025-07-29T10:26:01.010Z] tests/python/relax/test_transform_few_shot_tuning.py::test_funcs[False-Module] 2025-07-29 10:26:00 [INFO] LocalBuilder: max_workers = 16
[2025-07-29T10:26:06.253Z] PASSED
[2025-07-29T10:26:06.253Z] tests/python/relax/test_transform_few_shot_tuning.py::test_funcs[True-Softmax] 2025-07-29 10:26:06 [INFO] LocalBuilder: max_workers = 16 |
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 fixes an incorrect check for tuple nodes in the FunctionCreator class within the fuse_ops transformation. The issue was that the code was checking for tuple struct info before properly handling the tuple node case.
- Reorders the tuple checking logic to properly handle tuple nodes before checking struct info
- Moves the
partially_used_tuple_params_.erase()call to execute after tuple processing
src/relax/transform/fuse_ops.cc
Outdated
| if (arg.as<TupleNode>()) { | ||
| const Tuple& tup_args = Downcast<Tuple>(arg); | ||
| for (const Expr& tup_arg : tup_args->fields) { |
Copilot
AI
Jul 31, 2025
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.
The condition arg.as<TupleNode>() only checks if the argument is a TupleNode, but the subsequent code unconditionally downcasts to Tuple. This could cause a runtime error if the argument has TupleStructInfo but is not actually a TupleNode. Consider using if (auto tuple = arg.as<TupleNode>()) and use the tuple variable directly.
| if (arg.as<TupleNode>()) { | |
| const Tuple& tup_args = Downcast<Tuple>(arg); | |
| for (const Expr& tup_arg : tup_args->fields) { | |
| if (auto tuple = arg.as<TupleNode>()) { | |
| for (const Expr& tup_arg : tuple->fields) { |
* [Fix] Fix the wrong check for tuple node in apache#18163
This pr fix the wrong check for tuple node in #18163