-
Notifications
You must be signed in to change notification settings - Fork 39
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] Add benchmark for Buddy & OpenCV resize2d operation. #61
base: main
Are you sure you want to change the base?
Conversation
I think the latter is the expected behaviour. The way scaling ratios are defined is
So one would expect
This can be referenced from this comment here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am busy with my final semester exams atm, will do a more in-depth review as soon as I get some time.
Thanks for your work!
This benchmark will compare the resize operation for both Buddy & OpenCV implementation. Here is the problem, when using the SCALE_FACTOR option,Buddy used division operation;OpenCV used multiplication operation. The command I use: |
Another problem, when using SCALE_LENGTH option with the command like this: Result for OpenCV contains 400 * 300 pixels, result for Buddy contains 300 * 400 pixels. The dimension order is incorrect when handling with SCALE_LENGTH option. I also tried the SCALE_FACTOR option, this one have the right dimension order with both interpolation option. |
2911725
to
e17d20d
Compare
Both of these things can be changed if you want it to become exactly like |
I highly recommend to modify this part, thanks a lot! |
Will delete the OpenCV enum class after exams(before the weekend). |
Please mark the PR as "Ready for review" once you think it is ready. |
- Use clang-format for both resize benchmark files.
I think it makes sense to benchmark this function directly instead of the user interface itself. The user interface is just a single layer of syntactic sugar which makes it easier for users to utlize the underlying implementation without worrying about the output. It also involves costly "pass by value" situations which are very bad from the performance perspective. I wish to highlight the fact that this is not inherently required by the implementation itself, but is just added for more user convenience. Same goes for #63 as well. What is your opinion on this? (/cc @zhanghb97 @FlagerLee ) |
The command line:
./image-processing-resize-benchmark <image path> <Scale option> <RowNum> <ColNum> <InterpolationOption>
An example:
./image-processing-resize-benchmark ../../benchmarks/ImageProcessing/Images/YuTu.png SCALE_FACTOR 0.33 0.35 NEAREST_NEIGHBOUR_INTERPOLATION
Problem:
When using SCALE_FACTOR option, the output.size should be [input.row * RowNum, input.col * ColNum], but result shape ended in [input.row / RowNum, input.col / ColNum]. In the benchmark implementation, the factors(factorsOutputBuddyResize2D) were sent directly to the func below:
dip::Resize2D(&input, dip::INTERPOLATION_TYPE::NEAREST_NEIGHBOUR_INTERPOLATION, factorsOutputBuddyResize2D)
So, the problem was not caused by this implementation.