Skip to content

Add string input type support for SparseFillEmptyRows-16 #30802

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

p-wysocki
Copy link
Contributor

@p-wysocki p-wysocki commented May 30, 2025

Tickets:

Detailed Copilot summary

This pull request introduces support for string data types in the `SparseFillEmptyRows` operation across multiple components, including core functionality, tests, and plugins. The changes enhance the operation's versatility by allowing string values as inputs, outputs, and default values, along with corresponding updates to validation, execution logic, and test coverage.

Core functionality updates:

  • Removed the restriction on string inputs for SparseFillEmptyRows in the TensorFlow frontend (src/frontends/tensorflow_common/src/op/sparse_fill_empty_rows.cpp).
  • Added support for string data types in the execution logic of SparseFillEmptyRows (src/plugins/intel_cpu/src/nodes/sparse_fill_empty_rows.cpp).
  • Extended the evaluation logic to handle string data types (src/plugins/template/backend/ops/sparse_fill_empty_rows.cpp).

Test enhancements:

  • Added test cases for string data types in the C++ unit tests (src/core/tests/type_prop/sparse_fill_empty_rows.cpp).
  • Updated Python tests to include string data types in parameterized tests and input preparation logic (tests/layer_tests/tensorflow_tests/test_tf_SparseFillEmptyRows.py). [1] [2]
  • Introduced new test instances for string data types in CPU-specific functional tests (src/plugins/intel_cpu/tests/functional/custom/single_layer_tests/instances/common/sparse_fill_empty_rows.cpp).

Test data generation:

  • Implemented specialized test parameter generation for string data types in reference tests (src/plugins/template/tests/functional/op_reference/sparse_fill_empty_rows.cpp). [1] [2]

Additional updates:

  • Extended input generation logic in CPU tests to handle string values and default values (src/plugins/intel_cpu/tests/functional/custom/single_layer_tests/classes/sparse_fill_empty_rows.cpp). [1] [2] [3] [4]
  • Updated Python test files to include string data types in parameterized tests (src/bindings/python/tests/test_graph/test_sparse_fill_empty_rows.py).

p-wysocki added 2 commits May 30, 2025 13:34
Signed-off-by: p-wysocki <[email protected]>
Signed-off-by: p-wysocki <[email protected]>
@p-wysocki p-wysocki added this to the 2025.3 milestone May 30, 2025
@p-wysocki p-wysocki requested review from mitruska, rkazants and praasz May 30, 2025 11:46
@p-wysocki p-wysocki requested review from a team as code owners May 30, 2025 11:46
@p-wysocki p-wysocki requested review from akopytko and removed request for a team May 30, 2025 11:46
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: Python API OpenVINO Python bindings category: docs OpenVINO documentation category: TEMPLATE OpenVINO Template plugin category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd labels May 30, 2025
Copy link
Member

@rkazants rkazants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to implement SparseFillEmptyRows in a different way when string tensors are in unpacked format.
This packed format not well suits GPU and NPU devices where std::string is absent.

@p-wysocki
Copy link
Contributor Author

p-wysocki commented Jun 2, 2025

SparseFillEmptyRows for unpacked strings will be added as a separate operator due to otherwise convoluted API starting with #30812, but I suppose this PR stays valid? Or do we not expect regular string tensors to come?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: docs OpenVINO documentation category: Python API OpenVINO Python bindings category: TEMPLATE OpenVINO Template plugin category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants