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.

Sign up for GitHub

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

Rectify flipped coordinate_transformation_mode logic in ROIAlign #2159

Closed
wants to merge 6 commits into from
Closed

Rectify flipped coordinate_transformation_mode logic in ROIAlign #2159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
17bdf13
Rectify flipped coordinate_transformation_mode logic in ROIAlign
music-dino Sep 7, 2023
37b1ab0
Handle both opset 10 and 16 versions
music-dino Sep 18, 2023
e8a4082
Merge remote-tracking branch 'upstream/develop' into roialign_fix
music-dino Sep 18, 2023
b492968
Fix version check and clang tidy warning
music-dino Sep 18, 2023
bc474b9
Merge branch 'develop' into roialign_fix
causten Sep 19, 2023
ecc2a4a
Merge branch 'develop' into roialign_fix
causten Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/include/migraphx/op/roialign.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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{};
Expand Down
11 changes: 7 additions & 4 deletions src/onnx/parse_roialign.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,18 @@ struct parse_roialign : op_parser<parse_roialign>
std::vector<op_desc> 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<instruction_ref>& 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 +
Expand Down
2 changes: 1 addition & 1 deletion src/targets/gpu/jit/roialign.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct roialign_compiler : compiler<roialign_compiler>

// coord_trans_mode
auto ctm = v.at("coordinate_transformation_mode").to<std::string>();
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
Expand Down
8 changes: 7 additions & 1 deletion test/onnx/onnx_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
8 changes: 4 additions & 4 deletions test/ref/roialign.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<float> results_vector;
Expand Down Expand Up @@ -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<float> results_vector;
Expand All @@ -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<float> results_vector;
Expand All @@ -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<float> results_vector;
Expand Down
Loading