[PT FE] Support aten::take operation

[vijaykr338]

Details:

I was able to implement aten::take and aten::Delete function,

but aten::str posed really high difficult, i was only able to partially implement the it

currently, my implementation works for constant tensors, but for dynamic tensors, it currently stores shape and dtype for later use in debugging.

I have another question, how do you write tests for functions which are not implemented in Aten library yet?

I was able to write test cases for aten::take by following examples existing pytest files in the PyTorch layer tests folder.

but when I did the same for Delete and str, they consistently gave error

this is my first contribution to openVINO, I would greatly appreciate any guidance or assistance.

Tickets:

• [Good First Issue]: Support aten::Delete aten::str aten::take #28707

[mvafin]

Please fix code style and add tests for aten::str and aten::Delete.

[vijaykr338]

Please fix code style and add tests for aten::str and aten::Delete.

Sir @mvafin, I will fix the code style but

I am having issues writing tests for str

E       openvino._pyopenvino.OpConversionFailure: Check 'is_conversion_successful' failed at src/frontends/pytorch/src/frontend.cpp:174:
E       FrontEnd API failed with OpConversionFailure:
E       Model wasn't fully converted.
E       Summary:
E       -- No conversion rule found for operations: aten::str

I have added the aten::str in the op_table.cpp, along with the function name

[mvafin]

Please fix code style and add tests for aten::str and aten::Delete.

Sir @mvafin, I will fix the code style but

I am having issues writing tests for str

E       openvino._pyopenvino.OpConversionFailure: Check 'is_conversion_successful' failed at src/frontends/pytorch/src/frontend.cpp:174:
E       FrontEnd API failed with OpConversionFailure:
E       Model wasn't fully converted.
E       Summary:
E       -- No conversion rule found for operations: aten::str

I have added the aten::str in the op_table.cpp, along with the function name

Did you build openvino? It seems you are testing the release version of openvino, not the one you built.

[vijaykr338]

Please fix code style and add tests for aten::str and aten::Delete.

Sir @mvafin, I will fix the code style but
I am having issues writing tests for str

E       openvino._pyopenvino.OpConversionFailure: Check 'is_conversion_successful' failed at src/frontends/pytorch/src/frontend.cpp:174:
E       FrontEnd API failed with OpConversionFailure:
E       Model wasn't fully converted.
E       Summary:
E       -- No conversion rule found for operations: aten::str

I have added the aten::str in the op_table.cpp, along with the function name

Did you build openvino? It seems you are testing the release version of openvino, not the one you built.

I have recognized the issue
There was an issue with the registeration of aten::str in op_table.cpp, I am working on it now

[vijaykr338]

@mvafin I have updated the implementation and test file for aten::take and aten::delete
since there is no delete function in torch library, I assumed that it is meant to remove an element from input tensor, by slicing the portion before and after that element and then concatenating those slices

so my model looks like this for aten::delete

 def create_model(self):
        class aten_delete(torch.nn.Module):
            def forward(self, x, index):
                idx = int(index.item())
                # Delete along axis 0 by concatenating slices before and after idx.
                return torch.cat([x[:idx], x[idx+1:]], dim=0)
        ref_net = None
        return aten_delete(), ref_net, ["aten::slice", "aten::cat"]

please check

[vijaykr338]

for aten::Str, I'd like to confirm it's intended function

should it mimic python's inbuilt str() function? the general function is to convert tensors into string representation

my current implementation does not work, as it gives errors

E       openvino._pyopenvino.OpConversionFailure: Check 'is_conversion_successful' failed at src/frontends/pytorch/src/frontend.cpp:174:
E       FrontEnd API failed with OpConversionFailure:
E       Model wasn't fully converted. Failed operations detailed log:
E       -- prim::Constant with a message:
E       String constant cannot be converted to OpenVINO opset and should be removed by consuming operation.
E       Summary:
E       -- Conversion is failed for: prim::Constant

according to docs, raw string constants aren’t directly supported in the OpenVINO opset - instead I found StringTensorUnpack - which can make break the string into three parts to make it usable for further operations

[vijaykr338]

@mvafin Please let me know your thoughts when you have a moment. Thank you.

[mvafin]

@vijaykr338 Openvino support string tensors if created inside the model. However we cannot return or accept strings as model inputs/outputs. What model are you testing this on? I think it might be very hard or even impossible to make such a model that will be using aten::str inside.

As for delete, you are clearly not using aten::Delete inside the model code. I suspect that this operation is del that removes an element from dict. If so you wouldn't be able to implement since openvino doesn't support dictionaries, we decompose them.

[vijaykr338]

@vijaykr338 Openvino support string tensors if created inside the model. However we cannot return or accept strings as model inputs/outputs. What model are you testing this on? I think it might be very hard or even impossible to make such a model that will be using aten::str inside.

As for delete, you are clearly not using aten::Delete inside the model code. I suspect that this operation is del that removes an element from dict. If so you wouldn't be able to implement since openvino doesn't support dictionaries, we decompose them.

given such constraints, I think it would be wise to drop these 2 operations and focus on refining aten::Take instead

for aten::str, I thought about using an encoding-decoding pipeline to convert the string data to numerical data then back to it which is basically openVINO tokenizers, but that simply wont work.

similarly for aten::delete, I had another plan of making a 'mask' tensor with true and false values to filter out false indice elements from target tensor, but that wouldnt work either considering the constraints

I think it's better for me to refine just the aten::take function

[mvafin]

@mvafin @rkazants the tests for pytorch ran fine on the other operating systems, I am not sure why Ubuntu 22.04 failed, even the logs didn't help

It failed by timeout. Tests either too long or we just have added too many new tests

[vijaykr338]

@mvafin @rkazants the tests for pytorch ran fine on the other operating systems, I am not sure why Ubuntu 22.04 failed, even the logs didn't help

It failed by timeout. Tests either too long or we just have added too many new tests

I see

I have reduced the amount of tests

[vijaykr338]

@mvafin there seems to be some CI/CD issues, I checked the logs but they don't seem to be related to my implementation

[vijaykr338]

@mvafin @rkazants Just checking in — is there anything else I should change or address in this PR? I noticed that the CI failures don’t seem related to my implementation.

I would really appreciate it if you could take a look whenever you get a chance

thanks

[github-actions]

This PR will be closed in a week because of 2 weeks of no activity.

[mlukasze]

build_jenkins

[vijaykr338]

@mlukasze the tests have completed, could you take a look?

[mlukasze]

build_jenkins

[vijaykr338]

@mlukasze apologies for mentioning again, I believe that this PR can be merged now since all checks have passed successfully.

[mmikolajcz]

Suggested change

	auto input_shape = input.get_partial_shape();
	if (input_shape.rank().is_static() && input_shape.rank().get_length() == 0) {
	FRONT_END_OP_CONVERSION_CHECK(false, "input tensor MUST be non-scalar");
	}

Looking on how torch.take functions, scalar tensors should be allowed, for example:
torch.take(torch.tensor(5), torch.tensor(0)) -> tensor(5)
torch.take(torch.tensor(5), torch.tensor([0])) -> tensor([5])

Is there any reason for such check? I've modified test suite a bit and it seem that scalars should work correctly.

[mmikolajcz]

Please add test case for scenario with out. This would require few changes for whole class, you can refer to similar op: https://github.com/openvinotoolkit/openvino/blob/master/tests/layer_tests/pytorch_tests/test_take_along_dim.py#L51

[github-actions]

This PR will be closed in a week because of 2 weeks of no activity.

[github-actions]

This PR will be closed in a week because of 2 weeks of no activity.