[OpenVINO Backend] update getitem #21359

Mohamed-Ashraf273 · 2025-06-05T12:17:04Z

Hi @rkazants ,
I've updated __getitem__ using gather_nd as suggested, and I've avoided using convert_to_numpy. Also, I've enabled variables_test.
__getitem__ passed all tests aviable + I've tested it locally.
Here are my test cases:

import numpy as np
from keras import ops
from keras.src import testing

class GetItemOpsTest(testing.TestCase):
    def test_getitem(self):
        self.np_tensor = np.arange(24).reshape(2, 3, 4)
        self.tensor = ops.convert_to_tensor(self.np_tensor)

        t = self.tensor[1]
        n = self.np_tensor[1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, 2, 3]
        n = self.np_tensor[1, 2, 3]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2]
        n = self.np_tensor[1:2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, 2:3, 3:4]
        n = self.np_tensor[1:2, 2:3, 3:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, None]
        n = self.np_tensor[1:2, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, 2:3, ...]
        n = self.np_tensor[1:2, 2:3, ...]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2, ..., 3:4]
        n = self.np_tensor[1:2, ..., 3:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., 3:4, None]
        n = self.np_tensor[None, ..., 3:4, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1:2:None]
        n = self.np_tensor[1:2:None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[:, 2]
        n = self.np_tensor[:, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None]
        n = self.np_tensor[None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, None]
        n = self.np_tensor[None, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[...]
        n = self.np_tensor[...]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., 1]
        n = self.np_tensor[..., 1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., 1, 2]
        n = self.np_tensor[..., 1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., -1, 2]
        n = self.np_tensor[..., -1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., -1:-2, 2]
        n = self.np_tensor[..., -1:-2, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[..., None, None]
        n = self.np_tensor[..., None, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., None]
        n = self.np_tensor[None, ..., None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, 2, None, ..., None]
        n = self.np_tensor[1, 2, None, ..., None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[None, ..., 1, 2]
        n = self.np_tensor[None, ..., 1, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, None, 2]
        n = self.np_tensor[1, None, 2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(1, dtype=np.int32))
        t = self.tensor[index_tensor]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor)]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(1, dtype=np.int32))
        t = self.tensor[index_tensor, 2, None]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor), 2, None]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(-2, dtype=np.int32))
        t = self.tensor[index_tensor, 1]
        n = self.np_tensor[ops.convert_to_numpy(index_tensor), 1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        index_tensor = ops.convert_to_tensor(np.array(-1, dtype=np.int32))
        t = self.tensor[-2, index_tensor]
        n = self.np_tensor[-2, ops.convert_to_numpy(index_tensor)]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Negative indexing
        t = self.tensor[-1]
        n = self.np_tensor[-1]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[1, -1, -2]
        n = self.np_tensor[1, -1, -2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Slicing with step
        t = self.tensor[::2]
        n = self.np_tensor[::2]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        # Mixed slices and integers
        t = self.tensor[1, :, 1:4]
        n = self.np_tensor[1, :, 1:4]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

        t = self.tensor[:, 1:2, 3]
        n = self.np_tensor[:, 1:2, 3]
        self.assertEqual(t.shape, n.shape)
        self.assertTrue(np.array_equal(ops.convert_to_numpy(t), n))

The PR is ready for review.

codecov-commenter · 2025-06-05T12:23:05Z

Codecov Report

Attention: Patch coverage is 81.39535% with 16 lines in your changes missing coverage. Please review.

Project coverage is 82.74%. Comparing base (771b001) to head (58b23f3).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
keras/src/backend/openvino/core.py	81.39%	8 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #21359      +/-   ##
==========================================
+ Coverage   82.70%   82.74%   +0.04%     
==========================================
  Files         565      565              
  Lines       54894    54969      +75     
  Branches     8517     8534      +17     
==========================================
+ Hits        45398    45484      +86     
+ Misses       7415     7397      -18     
- Partials     2081     2088       +7

Flag	Coverage Δ
keras	`82.55% <81.39%> (+0.04%)`	⬆️
keras-jax	`63.48% <0.00%> (-0.09%)`	⬇️
keras-numpy	`58.64% <0.00%> (-0.08%)`	⬇️
keras-openvino	`33.79% <81.39%> (+0.26%)`	⬆️
keras-tensorflow	`63.87% <0.00%> (-0.09%)`	⬇️
keras-torch	`63.51% <0.00%> (-0.09%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rkazants · 2025-06-08T16:52:03Z

@Mohamed-Ashraf273, can you add your tests to sources as well?

rkazants · 2025-06-08T16:54:28Z

keras/src/backend/openvino/core.py

+                    )
+                if index_type != Type.i32:
+                    index = ov_opset.convert(index, Type.i32).output(0)
+                shape_tensor = ov_opset.shape_of(data)


you can specify output type to i32 then you don't need convert below.

If I do that index = get_ov_output(index, Type.i32) it will not be changed to i.32 based on the implementation of get_ov_ouput:

rkazants · 2025-06-08T16:55:19Z

keras/src/backend/openvino/core.py

+                index = ov_opset.select(
+                    is_negative, adjusted_index, index
+                ).output(0)
+                index_shape = index.get_partial_shape()


already computed above. Please clean-up your code properly.

I computed index_shape = index.get_partial_shape() above to ensure it's less than or equal to 1, else I would terminate early, the second computation, because the select operation can change the shape of index.
Based on this test case:

index_tensor = ops.convert_to_tensor(np.array(1, dtype=np.int32)) t = self.tensor[index_tensor] n = self.np_tensor[ops.convert_to_numpy(index_tensor)] self.assertEqual(t.shape, n.shape)

Fails due to the change of len(shape) to 0, and we need to unsqueeze.

Mohamed-Ashraf273 · 2025-06-08T17:58:05Z

@Mohamed-Ashraf273, can you add your tests to sources as well?

@rkazants
I added them to /keras/keras/src/ops/core_test.py

Mohamed-Ashraf273 · 2025-06-14T11:41:33Z

Hi @rkazants ,
I wanted to follow up and see if there are any updates.

google-ml-butler bot added the size:M label Jun 5, 2025

google-ml-butler bot assigned gbaned Jun 5, 2025

Mohamed-Ashraf273 force-pushed the update_getitem branch 5 times, most recently from 88938fd to 5043d78 Compare June 5, 2025 22:08

Mohamed-Ashraf273 marked this pull request as ready for review June 5, 2025 22:10

Mohamed-Ashraf273 marked this pull request as draft June 5, 2025 22:16

Mohamed-Ashraf273 force-pushed the update_getitem branch from 5043d78 to 91aa315 Compare June 5, 2025 22:40

Mohamed-Ashraf273 marked this pull request as ready for review June 5, 2025 22:40

Mohamed-Ashraf273 marked this pull request as draft June 5, 2025 22:49

Mohamed-Ashraf273 force-pushed the update_getitem branch 2 times, most recently from e7f4898 to 0ba9c10 Compare June 6, 2025 01:31

Mohamed-Ashraf273 marked this pull request as ready for review June 6, 2025 01:32

Mohamed-Ashraf273 force-pushed the update_getitem branch 3 times, most recently from d274a50 to 6ddb1b4 Compare June 6, 2025 12:30

Mohamed-Ashraf273 marked this pull request as draft June 6, 2025 12:34

Mohamed-Ashraf273 force-pushed the update_getitem branch 2 times, most recently from 4a1d931 to 08aebcf Compare June 6, 2025 20:26

Mohamed-Ashraf273 marked this pull request as ready for review June 6, 2025 20:27

rkazants reviewed Jun 8, 2025

View reviewed changes

[OpenVINO Backend] update getitem

47c145c

Mohamed-Ashraf273 force-pushed the update_getitem branch from 08aebcf to 47c145c Compare June 8, 2025 17:55

add tests for getitem method

58b23f3

Mohamed-Ashraf273 requested a review from rkazants June 11, 2025 12:05

google-ml-butler bot added the awaiting review label Jun 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[OpenVINO Backend] update getitem #21359

[OpenVINO Backend] update getitem #21359

Uh oh!

Mohamed-Ashraf273 commented Jun 5, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Jun 5, 2025 •

edited

Loading

Uh oh!

rkazants commented Jun 8, 2025

Uh oh!

rkazants Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Uh oh!

rkazants Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 14, 2025

Uh oh!

Uh oh!

[OpenVINO Backend] update __getitem__ #21359

Are you sure you want to change the base?

[OpenVINO Backend] update __getitem__ #21359

Uh oh!

Conversation

Mohamed-Ashraf273 commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rkazants commented Jun 8, 2025

Uh oh!

rkazants Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

rkazants Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Mohamed-Ashraf273 Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

Mohamed-Ashraf273 commented Jun 8, 2025

Uh oh!

Mohamed-Ashraf273 commented Jun 14, 2025

Uh oh!

Uh oh!

[OpenVINO Backend] update getitem #21359

[OpenVINO Backend] update getitem #21359

Mohamed-Ashraf273 commented Jun 5, 2025 •

edited

Loading

codecov-commenter commented Jun 5, 2025 •

edited

Loading