-
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
[PT FE] [23325] Add aten::masked_select support for pytorch models. #23354
Conversation
Hi openvino team,
|
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.
tests with GPU failed with:
Please ignore tests on GPU, you can run tests with TEST_DEVICE=CPU
tests with CPU failed with:
You have dynamic rank of input, that shouldn't happen usually since we set static input rank in tests. You could try adding trace_model=True
in your self._test
call or if that doesn't work you could use dynamic_shapes=False
, but that is less preferred.
@pytest.mark.parametrize( | ||
"mask_select", ['zeros', 'ones', 'random']) | ||
@pytest.mark.parametrize("input_dtype", [np.float32, np.float64, int, np.int32]) | ||
@pytest.mark.parametrize("mask_dtype", [np.uint8, np.int32]) # np.float32 incorrectly casted to bool |
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.
np.float32 is incorrectly casted by torch or openvino?
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 was just following test case in test_masked_fill.
https://github.com/openvinotoolkit/openvino/blob/master/tests/layer_tests/pytorch_tests/test_masked_fill.py#L67
Should I keep it or remove it?
Co-authored-by: Maxim Vafin <[email protected]>
Hi @mvafin, https://pytorch.org/docs/stable/generated/torch.masked_select.html What prevents cpu to support dynamic shapes? From the code, it looks like only the Result can be dynamic.
In plugins/intel_cpu/src/node.cpp:90, it requires shapes on outputs.
Here is the inputs/outputs shapes printed by test_masked_select:
|
@mvafin what do you think about it? |
const Output<Node>& data, | ||
const Output<Node>& mask) { | ||
auto _index = rg.make<opset10::NonZero>(mask); | ||
return rg.make<opset10::GatherND>(data, _index); |
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.
You need to do Transpose
after NonZero
, since the format of indices in GatherND
is different. Like here:
auto masked_id = context.mark_node(std::make_shared<v1::Transpose>(nonzero, input_order)); |
This PR will be closed in a week because of 2 weeks of no activity. |
This PR was closed because it has been stalled for 2 week with no activity. |
Details:
Tickets: