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.

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

[DIP] Make Resize API more consistent with OpenCV #154

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

meshtag
Copy link
Member

@meshtag meshtag commented Jun 2, 2023

This PR intends to make some changes in resize API to make it more consistent with OpenCV. Ref: buddy-compiler/buddy-benchmark#61 (comment)

@taiqzheng, can you confirm if these changes would help your work?

@meshtag meshtag added the DIP label Jun 2, 2023
@meshtag meshtag self-assigned this Jun 2, 2023
@taiqzheng
Copy link
Contributor

Sorry, i misunderstood OpenCV doc, its resize() read params in [width, height] format, instead of [h, w]. I 'm really sorry for this mistake, later i will revise the initial func in OpenCV benchmark design.

@taiqzheng
Copy link
Contributor

Shall i delete enum InterpolationOption in BuddyResize2DBenchmark.cpp(& OpenCVResize2DBenchmark.cpp), and use the one in DIP.h?

frontend/Interfaces/buddy/DIP/DIP.h Show resolved Hide resolved
}
std::reverse(scalingRatios.begin(), scalingRatios.end());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also adjust the OpenCV dimension(previous PR give [h, w], but it need [w, h]), so the dimension conversion is no longer required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand, we need to feed dimensions in [h, w] format at the lower level and we capture input in [w, h] format from the higher level.

Copy link
Contributor

@taiqzheng taiqzheng Jun 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmark code acquire inputs in [h, w], in the example <RowNum> comes before <ColNum>, so we capture high level inputs in [h, w] format:
./image-processing-resize-benchmark <image path> <Scale option> <RowNum> <ColNum> <InterpolationOption>

The lower level format is only determined by API(Buddy or OpenCV), so both of them need to convert [h, w] input in high level to [w, h] level in low level.

How about changing the benchmark code, i will do the [h, w] -> [w, h] conversion in the benchmark code, so the low level API get the [w, h] format. Also, the code in DIP.h dont need a switch anymore, the following code
return detail::Resize2D_Impl( input, type, {scalingRatios[1], scalingRatios[0]}, outputSize);
can change to
return detail::Resize2D_Impl( input, type, {scalingRatios[0], scalingRatios[1]}, outputSize); ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the benchmark code to take input in the [w, h] format.

Applying std::reverse makes it easier to read the code imo, because then we use scalingRatio[0] to get outputSize[0] and scalingRatio[1] to get outputSize[1].

Copy link
Contributor

@taiqzheng taiqzheng Jun 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the benchmark code to take input in the [w, h] format.

Please have a review at the resize2d PR. The tasks mentioned above were in the new commits.

@meshtag
Copy link
Member Author

meshtag commented Jun 5, 2023

its resize() read params in [width, height] format

So does this proposed API :)

Shall i delete enum InterpolationOption in BuddyResize2DBenchmark.cpp(& OpenCVResize2DBenchmark.cpp), and use the one in DIP.h?

Okay

@meshtag meshtag requested a review from taiqzheng June 6, 2023 15:10
@meshtag meshtag merged commit 3db23a6 into buddy-compiler:main Jun 8, 2023
@meshtag meshtag deleted the resize_api_change branch June 8, 2023 06:50
Guan-schoolmate pushed a commit to Guan-schoolmate/buddy-mlir that referenced this pull request Aug 14, 2023
Guan-schoolmate pushed a commit to Guan-schoolmate/buddy-mlir that referenced this pull request Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants