[TF FE] Support Div operation for TensorFlow

[sami0i]

Details:

• Support Div operation for TF

Tickets:

• Closes [Good First Issue]: Support Div operation for TensorFlow #21464

[sami0i]

I have modified div.cpp based on the translate_binary_op template. However, integrating this structure into the op_table requires the use of certain classes defined in the core. Thus, I changed it to {"Div",CreatorFunction(translate_binary_op(opset8::Divide)} in the op_table.
Currently, there is code for translate_floor_div_op in op_table.cpp. Can I remove div.cpp and implement the div operation in the same manner as translate_floor_div_op?

[rkazants]

Hi @sami0i,

Sorry for the delay. I see that you have failures in the layer tests for Div operations you added. The problem is related to inference results OV and TF for this operation mismatch.
The reason of it is that you use Divide operation of OV that uses floor division by default.
Please implement separate function helper (as you did initially) translate_div_op and use Divide with m_pythondiv = false. See documentation: https://docs.openvino.ai/2023.2/openvino_docs_ops_arithmetic_Divide_1.html

Best regards,
Roman

[rkazants]

Please address comment I left

[sami0i]

Hi @rkazants,
Thank you for your guidance.
I have a question: Should I delete the file div.cpp and place the translate_div_op function in the file binary_op.cpp?
Or, should I keep the file div.cpp and adapt the dictionary of Loaders based on that?

[rkazants]

Hi @rkazants, Thank you for your guidance. I have a question: Should I delete the file div.cpp and place the translate_div_op function in the file binary_op.cpp? Or, should I keep the file div.cpp and adapt the dictionary of Loaders based on that?

Let us use div.cpp and implement function helper there.

[rkazants]

build_jenkins

[rkazants]

Hi @sami0i,

Unfortunatelly, it still fails due to inference results mismatch between TF and OV:

E       AssertionError: Comparing with Framework failed: ie_res={'Div:0': array([[-1, -3, -1, -2, -1],
E              [-2,  3, -1, -3,  5],
E              [-1, -1,  1, -1, -1],
E              [ 6, -1, -2, -1, -5],
E              [-1, -1, -1,  0,  0],
E              [ 1, -2, -5,  0, -1],
E              [ 1,  0,  1, -2, -1],
E              [ 0,  6,  0, -1,  1]], dtype=int32), 'Div': array([[-1, -3, -1, -2, -1],
E              [-2,  3, -1, -3,  5],
E              [-1, -1,  1, -1, -1],
E              [ 6, -1, -2, -1, -5],
E              [-1, -1, -1,  0,  0],
E              [ 1, -2, -5,  0, -1],
E              [ 1,  0,  1, -2, -1],
E              [ 0,  6,  0, -1,  1]], dtype=int32)}; framework_res={'Div': array([[ 0, -3,  0, -1,  0],
E              [-1,  3,  0, -2,  5],
E              [ 0, -1,  1,  0,  0],
E              [ 6,  0, -2,  0, -5],
E              [-1,  0,  0,  0,  0],
E              [ 1, -2, -5,  0,  0],
E              [ 1,  0,  1, -1, -1],
E              [ 0,  6,  0,  0,  1]], dtype=int32)}.

Could you please investigate the logic for Div? And share examples when results diverge?
You can print generated operands in _prepare_input and share with me here. Now we understood that the single Divide does not fit well and more tricky sub-graph is needed for expression of Div.

Best regards,
Roman

[sami0i]

Hi @rkazants

It appears that the error occurred only when calculating the Div on int32 data types. I checked the tf.raw_ops.Div function, and it performs floor division for integer data types. Therefore, I think I should revert to setting m_pythondiv to true.

[rkazants]

Hi @rkazants

It appears that the error occurred only when calculating the Div on int32 data types. I checked the tf.raw_ops.Div function, and it performs floor division for integer data types. Therefore, I think I should revert to setting m_pythondiv to true.

m_pythondiv to true is also not quite correct because we implement translators to support all expected types in order to be fully aligned with TF.

Best regards,
Roman

[sami0i]

Hi @rkazants,

I've modified div.cpp according to your comments. Should I remove the print inputs_data['x'] and inputs_data['y'] lines from the test file?

[rkazants]

build_jenkins

[rkazants]

Hi @rkazants,

I've modified div.cpp according to your comments. Should I remove the print inputs_data['x'] and inputs_data['y'] lines from the test file?

Let us make sure that tests for int will pass and I will remove it. Hope to merge PR very soon.

Best regards,
Roman

[sami0i]

Hi @rkazants

I think It appears that <v1::Divide> behaves inconsistently across various os. For instance, when dividing -8 by 3, we anticipated <v1::Divide> to yield -3. We then added 1 to the expected result to obtain -2. However, it seems that the function initially calculated -2 accurately, and by adding 1, we obtained -1 as a result.

The solution that I think could be correct is to initially convert integer and signed inputs to floats. After calculating the division, we can then return the floor for positive values and the ceiling for negative values. Finally, we can convert the result back to an integer type.

[rkazants]

Hi @rkazants

I think It appears that <v1::Divide> behaves inconsistently across various os. For instance, when dividing -8 by 3, we anticipated <v1::Divide> to yield -3. We then added 1 to the expected result to obtain -2. However, it seems that the function initially calculated -2 accurately, and by adding 1, we obtained -1 as a result.

The solution that I think could be correct is to initially convert integer and signed inputs to floats. After calculating the division, we can then return the floor for positive values and the ceiling for negative values. Finally, we can convert the result back to an integer type.

Hi @sami0i,

It is a bug of OV that we have inconsistency for this op across different systems. Now I marked int32 test case as expected failed for Windows. I will report this bug to my colleagues for further fixing.

Regarding your proposal to use floating-point type, we may have a problem for big integers that some precision can be lost during conversion to float.

Best regards,
Roman

[rkazants]

Hi @rkazants

I think It appears that <v1::Divide> behaves inconsistently across various os. For instance, when dividing -8 by 3, we anticipated <v1::Divide> to yield -3. We then added 1 to the expected result to obtain -2. However, it seems that the function initially calculated -2 accurately, and by adding 1, we obtained -1 as a result.

The solution that I think could be correct is to initially convert integer and signed inputs to floats. After calculating the division, we can then return the floor for positive values and the ceiling for negative values. Finally, we can convert the result back to an integer type.

@sami0i, you are correct that Divide behaves inconsistently on different systems including both Windows and Linux. So I marked int32 case as expected failed.

[rkazants]

build_jenkins

[rkazants]

build_jenkins