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

[TF FE] Fix centernet and correct FloorDiv translator for signed integer type #22684

Merged
merged 31 commits into from Feb 14, 2024

Conversation

pavel-esir
Copy link
Contributor

@pavel-esir pavel-esir commented Feb 6, 2024

Details:

  • Centernet's topk operation returns large int32 values (greater than 1000000), even though they are integer FloorDiv/Div(inp_1, inp_2) + Floor operation is performed in float16 and because of that it causes accuracy problems.

  • To solve this need to performs FloorDiv operation in integer with a subgraph:

res = x / y; if x > 0 and y > 0
res = x / y - 1; if (x < 0 xor y < 0) and (x mod y != 0)
  • checked on separate bus: no degradations caused.

Tickets:

@github-actions github-actions bot added category: transformations OpenVINO Runtime library - Transformations category: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd labels Feb 6, 2024
@pavel-esir pavel-esir added the bug Something isn't working label Feb 6, 2024
@pavel-esir pavel-esir added this to the 2024.0 milestone Feb 6, 2024
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) and removed category: TFL FE OpenVINO TensorFlow Lite FrontEnd labels Feb 7, 2024
@pavel-esir pavel-esir marked this pull request as ready for review February 7, 2024 10:52
@pavel-esir pavel-esir requested review from a team as code owners February 7, 2024 10:52
@pavel-esir
Copy link
Contributor Author

@popovaan @rkazants i reverted removing of abs and instead added a separate tests cases where i statically specify inputs instead of random generation. I found that even for a very simple cases the current main, e.g.
FloorDiv(10, 10) which is equivalent to python 10 // 10 should be 1, but after compression of IR OpenVINO gives answer 0,

image

because if this denominator 10 is converted to Div(input_1, 10) => Mul(input_1, 1/10) then when 1/10 = 0.1 is compressed to f16 it becomes 0.099975.., which causes difference even for CPU inference!!! (because floor(10 * 0.099..) = floor(0.99..) = 0)

image
But after this fix works fine, since this constant is stored exactly without compression.

One more thing, since layer test call mo instead of ovc save_model is not called in layer tests, and therefore even with my fix tests appeared red. I added some changes to layer test class so that ovc is called, if these changes are radical i will revert them.

@github-actions github-actions bot removed category: GPU OpenVINO GPU plugin category: CPU OpenVINO CPU plugin category: transformations OpenVINO Runtime library - Transformations labels Feb 14, 2024
@pavel-esir
Copy link
Contributor Author

pavel-esir commented Feb 14, 2024

@alvoron FYI since Div for signed ints on Arm returns values different from cpu/gpu this FloorDiv also differ. xfailed Div and FloorDiv layer tests for arm until CVS-132377 is resolved.

@rkazants rkazants changed the title [TF FE] fix centernet [TF FE] Fix centernet and correct FloorDiv translator for signed integer type Feb 14, 2024
pavel-esir and others added 2 commits February 14, 2024 15:46
Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
pavel-esir and others added 2 commits February 14, 2024 16:53
Co-authored-by: Roman Kazantsev <roman.kazantsev@intel.com>
@rkazants rkazants added this pull request to the merge queue Feb 14, 2024
Merged via the queue into openvinotoolkit:master with commit 88b792e Feb 14, 2024
103 checks passed
@pavel-esir pavel-esir deleted the fix_centernet branch February 15, 2024 07:25
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: TF FE OpenVINO TensorFlow FrontEnd category: TFL FE OpenVINO TensorFlow Lite FrontEnd Code Freeze no-match-files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants