From 17bdf134cda9a27e8169d512671c6de27bc5cdbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dino=20Musi=C4=87?= Date: Thu, 7 Sep 2023 09:23:35 +0000 Subject: [PATCH 1/3] Rectify flipped coordinate_transformation_mode logic in ROIAlign --- src/include/migraphx/op/roialign.hpp | 2 +- src/targets/gpu/jit/roialign.cpp | 2 +- test/ref/roialign.cpp | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/include/migraphx/op/roialign.hpp b/src/include/migraphx/op/roialign.hpp index dda05b682f5..aa81cd89eab 100644 --- a/src/include/migraphx/op/roialign.hpp +++ b/src/include/migraphx/op/roialign.hpp @@ -125,7 +125,7 @@ struct roialign { xy[ii] = roi_start[ii] + p[ii] * bin_size[ii] + (i[ii] + .5f) * bin_size[ii] / bin_grid_size[ii]; - xy[ii] = (coord_trans_mode == "output_half_pixel") ? (xy[ii] - 0.5f) : xy[ii]; + xy[ii] = (coord_trans_mode == "half_pixel") ? (xy[ii] - 0.5f) : xy[ii]; if(xy[ii] < -1.0 or xy[ii] > dims[ii]) { results[index] = pos_weight{}; diff --git a/src/targets/gpu/jit/roialign.cpp b/src/targets/gpu/jit/roialign.cpp index 5da90c26621..c78ad1aeed9 100644 --- a/src/targets/gpu/jit/roialign.cpp +++ b/src/targets/gpu/jit/roialign.cpp @@ -81,7 +81,7 @@ struct roialign_compiler : compiler // coord_trans_mode auto ctm = v.at("coordinate_transformation_mode").to(); - float rois_offset = (ctm == "output_half_pixel") ? -0.5f : 0.0f; + float rois_offset = (ctm == "half_pixel") ? -0.5f : 0.0f; options.params += " -DROIS_OFFSET=" + std::to_string(rois_offset); // spatial_scale diff --git a/test/ref/roialign.cpp b/test/ref/roialign.cpp index 644b3cb43ca..1da031d46cc 100644 --- a/test/ref/roialign.cpp +++ b/test/ref/roialign.cpp @@ -73,7 +73,7 @@ TEST_CASE(roialign_out_of_bound_test) }; { - auto p = create_program("output_half_pixel"); + auto p = create_program("half_pixel"); p.compile(migraphx::make_target("ref")); auto result = p.eval({}).back(); std::vector results_vector; @@ -130,7 +130,7 @@ TEST_CASE(roialign_test) }; { - auto p = create_program(); + auto p = create_program("output_half_pixel"); p.compile(migraphx::make_target("ref")); auto result = p.eval({}).back(); std::vector results_vector; @@ -154,7 +154,7 @@ TEST_CASE(roialign_test) } { - auto p = create_program("output_half_pixel"); + auto p = create_program("half_pixel"); p.compile(migraphx::make_target("ref")); auto result = p.eval({}).back(); std::vector results_vector; @@ -175,7 +175,7 @@ TEST_CASE(roialign_test) } { - auto p = create_program("output_half_pixel", migraphx::op::pooling_mode::max, 0); + auto p = create_program("half_pixel", migraphx::op::pooling_mode::max, 0); p.compile(migraphx::make_target("ref")); auto result = p.eval({}).back(); std::vector results_vector; From 37b1ab00591572b6e89d01713dc00256c685a9ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dino=20Musi=C4=87?= Date: Mon, 18 Sep 2023 10:44:57 +0000 Subject: [PATCH 2/3] Handle both opset 10 and 16 versions --- src/onnx/parse_roialign.cpp | 11 +++++++---- test/onnx/onnx_test.cpp | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/onnx/parse_roialign.cpp b/src/onnx/parse_roialign.cpp index 88ccbde6105..822fb82f379 100644 --- a/src/onnx/parse_roialign.cpp +++ b/src/onnx/parse_roialign.cpp @@ -37,15 +37,18 @@ struct parse_roialign : op_parser std::vector operators() const { return {{"RoiAlign"}}; } instruction_ref parse(const op_desc& /*opd*/, - const onnx_parser& /*parser*/, + const onnx_parser& parser, onnx_parser::node_info info, const std::vector& args) const { - std::string coord_trans_mode = "half_pixel"; - if(contains(info.attributes, "coordinate_transformation_mode")) + std::string coord_trans_mode = + parser.opset_version == 16 ? "half_pixel" : "output_half_pixel"; + + if(const auto a = "coordinate_transformation_mode"; contains(info.attributes, a)) { - coord_trans_mode = info.attributes.at("coordinate_transformation_mode").s(); + coord_trans_mode = info.attributes.at(a).s(); } + if(not contains({"half_pixel", "output_half_pixel"}, coord_trans_mode)) { MIGRAPHX_THROW("coordinate_transformation_mode \"" + coord_trans_mode + diff --git a/test/onnx/onnx_test.cpp b/test/onnx/onnx_test.cpp index b23ace35073..2fa8d9e54e3 100644 --- a/test/onnx/onnx_test.cpp +++ b/test/onnx/onnx_test.cpp @@ -5899,7 +5899,13 @@ TEST_CASE(roialign_default_test) auto rois = mm->add_parameter("rois", srois); auto bi = mm->add_parameter("batch_ind", sbi); - auto r = mm->add_instruction(migraphx::make_op("roialign"), x, rois, bi); + // Due to the onnx model using opset 12, the coordinate_transformation_mode should be set to + // output_half_pixel + auto r = mm->add_instruction( + migraphx::make_op("roialign", {{"coordinate_transformation_mode", "output_half_pixel"}}), + x, + rois, + bi); mm->add_return({r}); auto prog = migraphx::parse_onnx("roialign_default_test.onnx"); From b4929684354324a7e475de4a011cfced1af78a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dino=20Musi=C4=87?= Date: Mon, 18 Sep 2023 15:18:39 +0000 Subject: [PATCH 3/3] Fix version check and clang tidy warning --- src/onnx/parse_roialign.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/onnx/parse_roialign.cpp b/src/onnx/parse_roialign.cpp index 822fb82f379..d397df608e3 100644 --- a/src/onnx/parse_roialign.cpp +++ b/src/onnx/parse_roialign.cpp @@ -42,9 +42,9 @@ struct parse_roialign : op_parser const std::vector& args) const { std::string coord_trans_mode = - parser.opset_version == 16 ? "half_pixel" : "output_half_pixel"; + parser.opset_version >= 16 ? "half_pixel" : "output_half_pixel"; - if(const auto a = "coordinate_transformation_mode"; contains(info.attributes, a)) + if(const auto* a = "coordinate_transformation_mode"; contains(info.attributes, a)) { coord_trans_mode = info.attributes.at(a).s(); }