Skip to content

Fix ReadValue variable memory by using fake alignment layout #30516

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 8 commits into
base: master
Choose a base branch
from

Conversation

yuanxion
Copy link
Contributor

@yuanxion yuanxion commented May 13, 2025

Details:

Use actual size of ReadValue variable memory to pass the ocl::check_boundaries checking during gpu_usm::copy_from().

Tickets:

@yuanxion yuanxion requested review from a team as code owners May 13, 2025 09:21
@github-actions github-actions bot added the category: GPU OpenVINO GPU plugin label May 13, 2025
@yuanxion
Copy link
Contributor Author

yuanxion commented May 13, 2025

For the florence2 notebook, the ReadValue node's variable memory layout 3x590x768 is smaller than the fake aligned memory layout 1776x1x768 (equals to 3x592x768) of its dependency FC node, and cannot pass the ocl::check_boundaries checking.
image

@yuanxion
Copy link
Contributor Author

It can pass the ocl::check_boundaries checking and run florence2 models successfully after applying the commit f3e1415.
Here is the object detection result:
image

However, this result is a little different with the result that does not use this PR and just comment out the ocl::check_boundaries checking, which has a door detected (need to check the diffenence later):
image

@yuanxion yuanxion force-pushed the fix-florence2-buffer-size branch from f3e1415 to ad1a761 Compare May 14, 2025 02:56
@yuanxion
Copy link
Contributor Author

Reverted previously fake aligned memory size for ReadValue, and just use the size of ReadValue variable's memory when copy_from its dependency memory.
image

@yeonbok
Copy link
Contributor

yeonbok commented May 14, 2025

Could you add a func test?

@yuanxion yuanxion force-pushed the fix-florence2-buffer-size branch from ad1a761 to 156c2b2 Compare May 15, 2025 01:53
@p-durandin p-durandin added the pr: needs tests PR needs tests updating label May 15, 2025
@p-durandin p-durandin added this to the 2025.2 milestone May 15, 2025
@yuanxion
Copy link
Contributor Author

Could you add a func test?

@yeonbok, @p-durandin Done, testcase is added.

Note: Google Test filter = variable_test_common.variable_copy_from_fc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from variable_test_common
[ RUN      ] variable_test_common.variable_copy_from_fc
[       OK ] variable_test_common.variable_copy_from_fc (343 ms)
[----------] 1 test from variable_test_common (343 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (344 ms total)
[  PASSED  ] 1 test.

@yuanxion yuanxion force-pushed the fix-florence2-buffer-size branch from b1593f9 to 833de62 Compare May 16, 2025 03:00
@yuanxion yuanxion force-pushed the fix-florence2-buffer-size branch from 5315779 to 1e8497f Compare May 22, 2025 02:39
@yeonbok yeonbok enabled auto-merge May 22, 2025 02:57
@yuanxion yuanxion force-pushed the fix-florence2-buffer-size branch from fa5156a to 9a8cc6c Compare May 23, 2025 08:05
@sshlyapn sshlyapn removed the pr: needs tests PR needs tests updating label May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: GPU OpenVINO GPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants