-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[onnx] Fix create ONNX DFT op for scalar signal size #32637
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
base: master
Are you sure you want to change the base?
[onnx] Fix create ONNX DFT op for scalar signal size #32637
Conversation
Signed-off-by: Raasz, Pawel <pawel.raasz@intel.com>
|
|
||
| bool dft_length_provided = !ov::op::util::is_null(length); | ||
| const bool dft_length_provided = !ov::op::util::is_null(length); | ||
| const auto unsqueeze_axis = v0::Constant::create(ov::element::i64, {}, {-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.
| const auto unsqueeze_axis = v0::Constant::create(ov::element::i64, {}, {-1}); | |
| const auto unsqueeze_axis = v0::Constant::create(ov::element::i32, {}, {0}); |
i32 is quite enough. Also, let use define axis, not -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.
Id like to same const in two cases and originally this const was i64.
But if this is ok to use i32 in both places I can change it
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.
Yes, i32 works and we waste less memory.
| const auto unsqueeze_axis = v0::Constant::create(ov::element::i64, {}, {-1}); | ||
| const auto& signal_size = [&] { | ||
| if (dft_length_provided) { | ||
| if (const auto& rank = length.get_partial_shape().rank(); rank.get_max_length() == 0) { |
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.
here you also need to check rank().is_static()
I propose to use Reshape operation with shape=[1] and apply this operation always without check. Or at least, apply Reshape when rank is dynamic.
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.
There is no need to check static, max length is 0 for scalar.
But in case if input is 2-D, reshape will change it to 1-D, but it should be still an error condition.
Details:
dft_lengthas scalar input which is not compatible with OV FFT operators (Must be 1-D). In case of scalar input unsqueeze this input to 1-D.Tickets: