Skip to content
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

[GPU] Init tensor.data when allocating inputs for string type #27269

Merged

Conversation

davidsnam-intel
Copy link
Contributor

@davidsnam-intel davidsnam-intel commented Oct 28, 2024

Details:

  • In case the element type is string produces the segmentation fault when input data is an empty string, unless the each element of tensor.data is initialized.

Tickets:

  • 148921

@davidsnam-intel davidsnam-intel requested review from a team as code owners October 28, 2024 09:06
@github-actions github-actions bot added category: Python API OpenVINO Python bindings category: TF FE OpenVINO TensorFlow FrontEnd labels Oct 28, 2024
@ilya-lavrenov ilya-lavrenov added the bug Something isn't working label Oct 28, 2024
@ilya-lavrenov ilya-lavrenov added this to the 2024.5 milestone Oct 28, 2024
Copy link
Contributor

@p-wysocki p-wysocki left a comment

Choose a reason for hiding this comment

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

All plugins handle empty strings without throwing but GPU, and this PR adds an additional if check for every string on all plugins. It's a high level fix for a low level issue.

Still, it's a fix for an existing issue, so I approve under the condition that is an additional story or discussion on what the desired plugin behavior for empty string is and adjust GPU if necessary.

Copy link
Contributor

@akuporos akuporos left a comment

Choose a reason for hiding this comment

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

I'd also expect to see the fix in GPU plugin directly, not in Python API.
As it was described in the ticket by @p-wysocki, the problem is GPU-specific, CPU works well.

@davidsnam-intel davidsnam-intel requested review from a team as code owners October 29, 2024 05:19
@davidsnam-intel davidsnam-intel changed the title [Python] Don't need to assign object to tensor for empty string [GPU] Initialize string type data with zero-charactor Oct 29, 2024
@github-actions github-actions bot added the category: GPU OpenVINO GPU plugin label Oct 29, 2024
@davidsnam-intel davidsnam-intel changed the title [GPU] Initialize string type data with zero-charactor [GPU] Initialize string type input data with zero-charactor Oct 29, 2024
@davidsnam-intel davidsnam-intel changed the title [GPU] Initialize string type input data with zero-charactor [GPU] Initialize string type input data with zero-character Oct 29, 2024
@davidsnam-intel davidsnam-intel force-pushed the david/fix-python-string branch from a39b4ab to c4843a4 Compare October 29, 2024 06:16
@github-actions github-actions bot removed the category: Python API OpenVINO Python bindings label Oct 29, 2024
@akuporos akuporos self-requested a review October 29, 2024 08:23
@davidsnam-intel davidsnam-intel force-pushed the david/fix-python-string branch from 179a200 to 57d56ac Compare October 29, 2024 08:49
@davidsnam-intel davidsnam-intel changed the title [GPU] Initialize string type input data with zero-character [GPU] Init tensor.data when allocating inputs for string type Oct 30, 2024
@davidsnam-intel davidsnam-intel force-pushed the david/fix-python-string branch 2 times, most recently from d3b882f to 484c727 Compare October 30, 2024 07:49
@isanghao isanghao dismissed akuporos’s stale review October 30, 2024 07:55

GPU implementation was fixed to resolve the issue.

@isanghao isanghao enabled auto-merge October 30, 2024 07:55
@isanghao isanghao added this pull request to the merge queue Oct 30, 2024
Merged via the queue into openvinotoolkit:master with commit a7ff891 Oct 30, 2024
187 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working category: GPU OpenVINO GPU plugin category: TF FE OpenVINO TensorFlow FrontEnd Code Freeze
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants