[CVS-172796] fix bfloat16 conversion when single cast node to bfloat16 #841
Conversation
…use original bf16 outputs tensor
bopeng1234
changed the title
…ansform for OV CPU
mklimenk
left a comment
mklimenk
left a comment
There was a problem hiding this comment.
Limited the condition of bfloat16 conversion to trigger only on GPU and NPU, since CPU has some sort of reference implementation.
Looks good to me, thanks.
|
@bopeng1234 . This simply ignores BF16-> FP16 for CPU. Why is this conversion needed for GPU and NPU? |
@mklimenk , can you help to clarify on it? |
|
As we discussed, removing the bfloat16 model conversion branch. |
|
@bopeng1234, please modify the onnxruntime/core/providers/openvino/ov_versions/data_ops.cc file as well to remove the bfloat16 handling there. You can refer to the original PR to see what's required |
|
Sure, revert the changes of #741 |
There was a problem hiding this comment.
Pull Request Overview
This PR reverts bfloat16 to float16 conversion functionality that was causing test failures. The transformation was converting single-node graphs in unexpected ways, particularly affecting Cast operation unit tests where the output type was changed from bfloat16 to float16.
Key Changes:
- Removed the entire bfloat16_fix transformation infrastructure
- Deleted associated test file for bfloat16 pass validation
- Removed bfloat16 type support from the type checking logic
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| openvino_ep_bfloat16_pass_test.cc | Deleted test file that validated bfloat16 model conversion behavior |
| qdq_scales_fix.h | Removed bfloat16_fix namespace declaration |
| qdq_scales_fix.cpp | Removed complete bfloat16_fix implementation including Transform function and conversion logic |
| data_ops.cc | Removed bfloat16 from supported types list |
| backend_manager.cc | Removed IsModelBF16 helper function and bfloat16 transformation path from model processing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MayureshV1
left a comment
MayureshV1
left a comment
There was a problem hiding this comment.
Removing Bf16 -> fp16 transformation
Looks good !
3 participants
Description
Disable bfloat16 model conversion when the graph's node size <=1.
Motivation and Context
CVS-172796
bfloat16_fix::Transform made changes to the bfloat16 output model, changed the output type to float16.
When single node test, cast_op_test.cc, it only has one node "Cast", the output is changed to float16 by the transformation, but testcase still use it as bfloat16 tensor, which will cause a mismatch;
May need to disable the transformation when unit test runs. OR. Add a cast node before BF16 output node.
This PR
revert the changes of #741, remove the bfloat16 model transformation