[SOT] Fallback when breakgraph in for loop#76104
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull Request Overview
This PR removes the loop breakgraph logic from the SOT (Symbolic Opcode Translator) and simplifies the FOR_ITER handling by always falling back instead of attempting to break the graph. The changes eliminate complex loop reconstruction code and replace it with simpler fallback behavior.
- Removed the
_break_graph_when_for_loopmethod and associated loop breakgraph resume function logic - Simplified
FOR_ITERto always fallback onBreakGraphErrorinstead of attempting loop reconstruction - Updated test files to use
@strict_mode_guard(False)decorator for tests that now require fallback behavior - Cleaned up enum values and removed unused imports
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| python/paddle/jit/sot/opcode_translator/executor/opcode_executor.py | Removed _break_graph_when_for_loop method (~250 lines), simplified FOR_ITER error handling to always fallback, removed unused imports, cleaned up formatting |
| python/paddle/jit/sot/opcode_translator/executor/pycode_generator.py | Removed unused ResumeFunctionType enum values for loop breakgraph and renumbered remaining values |
| test/sot/test_12_for_loop.py | Added @strict_mode_guard(False) to multiple test methods, removed decorator from function definition, renamed test method |
| test/sot/test_min_graph_size.py | Added strict_mode_guard import and decorator to test class method |
| test/sot/test_inplace_api.py | Added strict_mode_guard import and decorator to test method |
| test/sot/test_builtin_zip.py | Added @strict_mode_guard(False) decorator to test method |
Comments suppressed due to low confidence (1)
test/sot/test_builtin_zip.py:101
- [nitpick] Inconsistent decorator ordering: in
test_reconstructmethod (line 95-96),@min_graph_size_guardis placed before@strict_mode_guard, but intest_zip_user_defined_iter(line 100-101), the order is reversed. Consider using a consistent decorator ordering throughout the test class for better readability.
@strict_mode_guard(False)
@min_graph_size_guard(0)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def get_compute_fn_and_update_changed_vars( | ||
| self, restore_names, stack, end_idx, extra_store_vars | ||
| self, restore_names, stack, end_idx |
There was a problem hiding this comment.
The docstring for this method still references the removed extra_store_vars parameter. The docstring at line 2215 should be updated to remove the mention of extra_store_vars: for iterator, we need store the holder if it is a Tensor.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #76104 +/- ##
===========================================
Coverage ? 100.00%
===========================================
Files ? 2
Lines ? 4
Branches ? 0
===========================================
Hits ? 4
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR Category
Execute Infrastructure
PR Types
Performance
Description
SOT 移除针对 for-loop 的 break graph 处理，在 for-loop 中产生的 break graph 往往会产生大量的碎子图，反而会导致性能降低
此外，for-loop 的实现不再需要抽取函数并 inline call，后续移除