[GPU] Keep NMS boundary-sensitive subgraph in FP32 to fix Detectron2 FasterRCNN GPU extra detection by yuanxion · Pull Request #36723 · openvinotoolkit/openvino

yuanxion · 2026-07-05T09:01:04Z

Details

fixed a GPU accuracy regression in the Detectron2 Faster RCNN E2E case where GPU produces 17 detections while the reference output contains 16.

Description of the issue

Symptom

The Detectron2 Faster R-CNN E2E testcase on GPU shows a detection count mismatch: Reference output (16 detections) vs GPU output (17 detections). The failure happens in the final object-detection comparison stage.

Root cause

The extra GPU detection is not a random corruption or environment issue. It is a stable output difference caused by boundary-sensitive suppression behavior in the NMS tail. FP16 rounding in NMS can slightly change the comparison and cause one extra box to survive.

How to fix it

Use a new GPU matcher pass KeepNMSBoundaryPrecision to disables FP16 conversion NonMaxSuppression_1.

Reproduction step and snapshot

python -m pytest test_ovc_mo.py --tb=native --env_conf=.automation/env_config.yml --test_conf=.automation/test_configs/desktop_test_config_gpu_llm.yml -m "not launch_only_if_manually_specified" --pregen_irs=python_api_serialized/irs_mapping.csv --tf_models_version=1.15.2 --modules pipelines/production/pytorch/light/detectron2_faster_rcnn.py --dynamism_type=None --log-cli-level INFO

Problematic graph

Checklist

Is it a proper fix? (not a workaround)
Did you include test case for this fix, if necessary?
Did you review existing test that can be extended to cover this scenario? Which test did you review?

Tickets:

CVS-185940

AI Assistance:

AI assistance used: yes
AI find the root cause and human verify it

…s 16) Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

Copilot

Pull request overview

This PR addresses a GPU accuracy regression in Detectron2 Faster R-CNN by preventing FP16 precision conversion on a boundary-sensitive NonMaxSuppression (NMS) tail subgraph, keeping that region in FP32 to avoid suppression differences that can lead to an extra detection.

Changes:

Introduces a new GPU MatcherPass (KeepNMSBoundaryPrecision) that disables FP16 precision conversion for the targeted NMS subgraph.
Registers the pass in the Intel GPU transformations pipeline before ConvertPrecision.
Adds unit tests validating that the intended nodes are marked, and that non-targeted NMS nodes are not affected.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/plugins/intel_gpu/tests/unit/transformations/keep_nms_boundary_precision_test.cpp	Adds unit coverage for the new precision-keeping matcher pass.
src/plugins/intel_gpu/src/plugin/transformations/keep_nms_boundary_precision.hpp	Declares the new `KeepNMSBoundaryPrecision` matcher pass.
src/plugins/intel_gpu/src/plugin/transformations/keep_nms_boundary_precision.cpp	Implements graph matching and FP16 conversion disabling for the NMS boundary-sensitive chain.
src/plugins/intel_gpu/src/plugin/transformations_pipeline.cpp	Wires the new pass into the GPU transformation pipeline before `ConvertPrecision`.

Lyamin-Roman · 2026-07-07T16:20:22Z

+namespace ov::intel_gpu {
+namespace {
+
+constexpr char kTargetNmsPrefix[] = "torchvision::nms/";


I'm not sure relying on the operation name is the best idea
Are there any other special differences in the subgraph around the operation for this model?
Or maybe it's possible to change something in the cl kernel itself for more stable operation without significantly affecting the performance

Yes, I agree. I changed it to use a matches_batched_nms_chain and check specific ops in this model instead.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

Lyamin-Roman · 2026-07-09T12:15:45Z

+           ov::is_type<ov::op::v1::Reshape>(node) || ov::is_type<ov::op::v0::Unsqueeze>(node);
+}
+
+void mark_fp32_chain(const std::shared_ptr<ov::Node>& node,


Is it possible to use ov::mark_as_precision_sensitive instead of custom functions?

Lyamin-Roman · 2026-07-09T12:16:52Z

+    register_matcher(matcher, callback);
+}
+
+}  // namespace ov::intel_gpu


nit: Please, add a empty line to the end of each new file

Thanks, fixed.

Lyamin-Roman · 2026-07-09T12:25:49Z

+    ov::matcher_pass_callback callback = [this, boxes_offset_add_m, boxes_reshape_m, scores_unsqueeze_m, nms_m](Matcher& m) {
+        const auto& pattern_map = m.get_pattern_value_map();
+
+        auto nms = ov::as_type_ptr<ov::op::v9::NonMaxSuppression>(pattern_map.at(nms_m).get_node_shared_ptr());


Will the NMS version always be op::v9?
Apparently, the transformation is triggered before all NMS conversions occur src/plugins/intel_gpu/src/plugin/transformations_pipeline.cpp:997

Lyamin-Roman · 2026-07-09T18:04:10Z

As far as I understand, this shift in the NMS coordinates in the model is needed to emulate a Multiclass NMS.
Ideally, it would probably detect this pattern and transform it into a real Multiclass NMS. Then, there wouldn't be any problems with fp16 due to the artificial shift, and performance wouldn't suffer from using fp32.
But these are my assumptions, and perhaps such a transformation would be useful in the general context.

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

mitruska · 2026-07-13T16:57:10Z

+        const auto has_only_static_dimensions = [](const PartialShape& shape) {
+            return shape.rank().is_static() &&
+                   std::all_of(shape.begin(), shape.end(), [](const Dimension& dimension) {
+                       return dimension.is_static();
+                   });
+        };
+        if (!has_only_static_dimensions(data.get_partial_shape()) ||
+            !has_only_static_dimensions(indices.get_partial_shape())) {


This change and introduced lambda !has_only_static_dimensions is equivalent to the previous !is_dynamic() check. Should be reverted as not needed.

Reverted, thanks.

mitruska · 2026-07-13T17:10:42Z

+        const auto& data_shape = gather->get_input_partial_shape(0);
+        if (data_shape.rank().is_dynamic() ||
+            std::any_of(data_shape.begin(), data_shape.end(), [](const Dimension& dimension) {
+                return dimension.is_dynamic();
+            })) {
+            return false;
+        }
+


This is exactly what is_dynamic() check does, moreover, static shape is ensured here by {any_input(has_static_shape()) in the gather_label pattern above L134.

Also reverted.

pkowalc1 · 2026-07-14T06:47:16Z

Not sure what the bug really is("rounding fp16" error is too enigmatic for me), BUT if this bug is only related to numerical stability, it means that any nms can suffer from it. In this case, instead of having a special transformation for some cases, why won't we either:
a) Fix the operator itself, by e.g. doing all calculations internally in fp32?
b) Fix the operator by only accepting fp32/fp64 inputs, which I guess will effectively force a)?

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

Lyamin-Roman · 2026-07-14T14:28:11Z

Not sure what the bug really is("rounding fp16" error is too enigmatic for me), BUT if this bug is only related to numerical stability, it means that any nms can suffer from it. In this case, instead of having a special transformation for some cases, why won't we either: a) Fix the operator itself, by e.g. doing all calculations internally in fp32? b) Fix the operator by only accepting fp32/fp64 inputs, which I guess will effectively force a)?

It appears the problem is caused by the box coordinates shifting to the edge of the FP16 range, when the interval between adjacent values becomes larger, and not a problem with some FP16 accumulator (@yuanxion, correct me if I'm wrong)
If we get rid of this "hack" with Multiclass NMS emulation, this problem will resolve itself
And the solution of forcing FP32 will inevitably lead to performance degradation, the only question is how much
If there is an option to preserve the performance and restore the accuracy, I think we should choose this option

pkowalc1 · 2026-07-15T08:11:38Z

Not sure what the bug really is("rounding fp16" error is too enigmatic for me), BUT if this bug is only related to numerical stability, it means that any nms can suffer from it. In this case, instead of having a special transformation for some cases, why won't we either: a) Fix the operator itself, by e.g. doing all calculations internally in fp32? b) Fix the operator by only accepting fp32/fp64 inputs, which I guess will effectively force a)?

It appears the problem is caused by the box coordinates shifting to the edge of the FP16 range, when the interval between adjacent values becomes larger, and not a problem with some FP16 accumulator (@yuanxion, correct me if I'm wrong) If we get rid of this "hack" with Multiclass NMS emulation, this problem will resolve itself And the solution of forcing FP32 will inevitably lead to performance degradation, the only question is how much If there is an option to preserve the performance and restore the accuracy, I think we should choose this option

pkowalc1 · 2026-07-15T08:27:50Z

Once again, I don't know root cause of the problem, but if is related to numerical stability, then basically there are two options:

(harder) use better, numerically stable algorithm to perform calculation
(easier) perform calculations with higher precision and better accuracy -> I suggested this solution last time.

Note that if this is only related to some internal calculations, all of them can be performed using fp32 internally in the kernel(so input data is fp16, but calculations are done inside kernel using fp32) - this should not affect the performance in any measurable way, except for the case where kernel is already under register pressure and starts to spill regs to global mem - but then there are other solutions to fix it.

Note that if the problem is truly related to numerical stability of the kernel, this PR does not solve that problem - it only mitigates it for some cases.

About speed/performance - IMO it doesn't matter if the kernel produces wrong outputs. First we have to make sure kernel is correct and then eventually optimize it. In this case, it looks like the kernel is not correct for fp16 data type, so I suggest to try to fix the kernel, not to introduce some new transformations.

This is only suggestion, the decision is up to @yuanxion what to do with it.

pkowalc1 · 2026-07-15T08:28:16Z

(sorry for closing and reopening this PR - done by mistake)

yuanxion · 2026-07-15T09:39:08Z

Once again, I don't know root cause of the problem, but if is related to numerical stability, then basically there are two options:

(harder) use better, numerically stable algorithm to perform calculation

(easier) perform calculations with higher precision and better accuracy -> I suggested this solution last time.

Note that if this is only related to some internal calculations, all of them can be performed using fp32 internally in the kernel(so input data is fp16, but calculations are done inside kernel using fp32) - this should not affect the performance in any measurable way, except for the case where kernel is already under register pressure and starts to spill regs to global mem - but then there are other solutions to fix it.

Note that if the problem is truly related to numerical stability of the kernel, this PR does not solve that problem - it only mitigates it for some cases.

About speed/performance - IMO it doesn't matter if the kernel produces wrong outputs. First we have to make sure kernel is correct and then eventually optimize it. In this case, it looks like the kernel is not correct for fp16 data type, so I suggest to try to fix the kernel, not to introduce some new transformations.

This is only suggestion, the decision is up to @yuanxion what to do with it.

Thanks, both comments make sense.

Based on the debugging results, I believe this issue is caused by the batched NMS emulation pattern, not by an FP16 accumulator inside the NMS kernel. The class-dependent coordinate shift can push box values into an FP16-sensitive range, which makes the NMS decisions unstable. Replacing the emulation with real MulticlassNMS removes that failure mode directly.

On performance, the model-level impact is negligible: MulticlassNMS is 125.17 ms vs 124.74 ms for the old NMS chain FP32 subgraph. I could not obtain reliable kernel/subgraph timing for this dynamic-shape GPU case with the current profiling paths.

The MulticlassNMS transformation restores accuracy and preserves performance at model level. If we later identify a separate numerical issue inside the kernel itself, we can address that independently.

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

fix different number of detected objects for layer 'detections' (17 v…

e488ac5

…s 16) Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

yuanxion requested review from a team as code owners July 5, 2026 09:01

github-actions Bot added the category: GPU OpenVINO GPU plugin label Jul 5, 2026

yuanxion requested a review from Copilot July 7, 2026 02:39

Copilot started reviewing on behalf of yuanxion July 7, 2026 02:39 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread src/plugins/intel_gpu/src/plugin/transformations/keep_nms_boundary_precision.cpp

Comment thread src/plugins/intel_gpu/tests/unit/transformations/keep_nms_boundary_precision_test.cpp

p-durandin assigned Lyamin-Roman Jul 7, 2026

Lyamin-Roman reviewed Jul 7, 2026

View reviewed changes

yuanxion and others added 2 commits July 8, 2026 14:41

Apply suggestions from code review

89ef2bf

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Update nms match chain to keep FP32 precision

d6bf4d3

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

Lyamin-Roman reviewed Jul 9, 2026

View reviewed changes

convert batched nms to multiclass nms

e692b41

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

yuanxion requested review from a team as code owners July 13, 2026 08:56

github-actions Bot added category: Core OpenVINO Core (aka ngraph) category: transformations OpenVINO Runtime library - Transformations labels Jul 13, 2026

t-jankowski requested review from mitruska and removed request for a team July 13, 2026 12:34

mitruska reviewed Jul 13, 2026

View reviewed changes

add empty line to the end of new file

14d1239

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

pkowalc1 closed this Jul 15, 2026

pkowalc1 reopened this Jul 15, 2026

revert unnecessary code

b2942d5

Signed-off-by: yuan.xiong <yuan.xiong@intel.com>

Uh oh!

Conversation

yuanxion commented Jul 5, 2026

Details

Description of the issue

Symptom

Root cause

How to fix it

Reproduction step and snapshot

Problematic graph

Checklist

Tickets:

AI Assistance:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lyamin-Roman commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkowalc1 commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lyamin-Roman commented Jul 14, 2026

Uh oh!

pkowalc1 commented Jul 15, 2026

Uh oh!

pkowalc1 commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkowalc1 commented Jul 15, 2026

Uh oh!

yuanxion commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pkowalc1 commented Jul 14, 2026 •

edited

Loading

pkowalc1 commented Jul 15, 2026 •

edited

Loading