[SOT][FasterGuard] ShapeMatchGuard support dynamic shape
#72564
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the ShapeMatchGuard to support dynamic shapes for performance optimizations.
- Refactors the constructor to accept a vector of py::object and converts it to int64_t values.
- Modifies the shape-checking logic in the guard implementation.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| paddle/fluid/pybind/sot/guards.h | Adjusted constructor and changed internal type usage. |
| paddle/fluid/pybind/sot/guards.cc | Updated shape checking logic to validate non-positive dimensions. |
paddle/fluid/pybind/sot/guards.cc
Outdated
| @@ -141,7 +141,7 @@ bool ShapeMatchGuard::check(PyObject* value) { | |||
| return false; | |||
| } | |||
| for (size_t i = 0; i < shape.size(); ++i) { | |||
| if (expected_[i] && shape[i] != *expected_[i]) { | |||
| if (shape[i] <= 0 || shape[i] != expected_[i]) { | |||
There was a problem hiding this comment.
The updated condition checks for non-positive values and fails dynamic shapes that could be represented by a negative or zero value. To support dynamic shapes, consider revising the condition to allow for dynamic dimensions (e.g., checking if expected_[i] is set to a dynamic indicator such as -1 before comparing equality).
| if (shape[i] <= 0 || shape[i] != expected_[i]) { | |
| if (shape[i] <= 0 || (expected_[i] != -1 && shape[i] != expected_[i])) { |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Pull Request Overview
This PR adds dynamic shape support to the ShapeMatchGuard, improving performance by allowing flexible shape validation.
- Change the ShapeMatchGuard constructor to accept a vector of py::object and represent dynamic dimensions with a sentinel value.
- Replace the std::optional<int64_t> storage with std::vector<int64_t>, using -1 as the indicator for dynamic dimensions.
- Update the shape-check logic in guards.cc accordingly.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| paddle/fluid/pybind/sot/guards.h | Updated constructor and internal representation to support dynamic shape |
| paddle/fluid/pybind/sot/guards.cc | Modified shape checking logic to accommodate dynamic shape representation |
| if (py::isinstance<py::int_>(s)) { | ||
| expected_.push_back(s.cast<int64_t>()); | ||
| } else { | ||
| expected_.push_back(-1); |
There was a problem hiding this comment.
[nitpick] Consider replacing the magic number -1 with a named constant (e.g., kDynamicDimension) to improve code readability and maintainability.
| expected_.push_back(-1); | |
| expected_.push_back(kDynamicDimension); |
Copilot uses AI. Check for mistakes.
| @@ -141,7 +141,8 @@ bool ShapeMatchGuard::check(PyObject* value) { | |||
| return false; | |||
| } | |||
| for (size_t i = 0; i < shape.size(); ++i) { | |||
There was a problem hiding this comment.
[nitpick] Adding a brief comment clarifying that -1 is used as a sentinel for dynamic dimensions would help maintainers understand the control flow.
| for (size_t i = 0; i < shape.size(); ++i) { | |
| for (size_t i = 0; i < shape.size(); ++i) { | |
| // -1 is used as a sentinel value for dynamic dimensions, allowing any positive size to match. |
Copilot uses AI. Check for mistakes.
PR Category
Performance Optimization
PR Types
Performance
Description
ShapeMatchGuard支持动态 shape