-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Allow all single-element tensors as value for ConstantOfShape #26227
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
Updated enforcement conditions to allow any dimensional tensor with a single element according to the ONNX spec.
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 relaxes the previous restriction that the ConstantOfShape "value" attribute must be a 1-D tensor of size 1, aligning behavior with the ONNX spec allowing any shape as long as it contains exactly one element.
- Removed hard enforcement of dims_size()==1 and dims()[0]==1
- Added a comment indicating intent to allow any dimensional single-element tensor
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
0f487e9
to
bd1f869
Compare
Signed-off-by: Justin Chu <[email protected]>
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin Chu <[email protected]>
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
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.
LGTM!
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
I agree with the suggestion to use a range-based for loop. Since we are using C++17, a range-based for loop would be more modern and readable than For example: int64_t size = 1;
for (auto dim : t_proto_p->dims()) {
size *= dim;
} This is also in line with suggestions from other reviewers. |
Signed-off-by: Justin Chu <[email protected]>
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.
onnxruntime/core/providers/cpu/generator/constant_of_shape_base.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Justin Chu <[email protected]>
Signed-off-by: Justin Chu <[email protected]>
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.
Updated enforcement conditions to allow any dimensional tensor with a single element according to the ONNX spec.
Fix #26218 Fix #26265