-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[PyTorch] : implement support for replicated{1,2,3} pad #28271
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: 11happy <[email protected]>
humble ping! |
auto data = context.get_input(0); | ||
auto paddings = context.const_input<std::vector<int64_t>>(1); | ||
Output<Node> pad_value = context.mark_node(v0::Constant::create(element::f32, Shape{}, {0})); | ||
return translate_pad_common(context, data, paddings, pad_value, "replicate"); |
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.
I am not sure that logic from translate_pad_common
is suitable for replication
operation.
That is because a format of paddings is as follows <pad_begin_axis1>, <pad_end_axis1>, <pad_begin_axis2>, <pad_end_axis2>, ...
in case of tuple.
May be we need to do as follows:
- do broadcast of padding to convert to
tuple
case - create a pad vector with zeros with length equal to rank of
input
- scatter elements from broadcasted padding to
pad
vector with known indices defined for each 1d, 2d and 3d cases
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.
I am not able to currently get why this logic maybe incorrect can you please give an example to explain. it would be very helpful. Thank you
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.
Hi @11happy,
I meant logic for preparation of pad vector can be simplified but I understand it may be out-of-scope for this PR because it is used by other translators.
Let us see tests if they pass. please resolve conflicts and I will trigger CI for your PR.
Thanks.
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.
resolved the conflicts.
Thank you
build_jenkins |
Signed-off-by: 11happy <[email protected]>
Signed-off-by: 11happy <[email protected]>
build_jenkins |
Overview:
This pull request fixes #23322.
Testing:
CC: