Skip to content

Erode and Dilate - Tensor based optimization for hip#80

Merged
kiritigowda merged 10 commits intoROCm:masterfrom
r-abishek:ar/opt_erode_dilate
Dec 3, 2021
Merged

Erode and Dilate - Tensor based optimization for hip#80
kiritigowda merged 10 commits intoROCm:masterfrom
r-abishek:ar/opt_erode_dilate

Conversation

@r-abishek
Copy link
Copy Markdown
Member

  • Adds erode and dilate in tensor for hip
  • Adds corresponding unit and performance tests

@asalmanp This PR contains tensor based optimized erode and dilate implementations for hip.

test_case_name = "erode";

Rpp32u kernelSize = additionalParam;
for (i = 0; i < images; i++)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what's the purpose of this for loop here? the entire body of the for loop is commented out!. please remove it here and for all of the new unit tests that have a similar for-loop.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Modified comment for better clarity on the use case of the ROI override.

Copy link
Copy Markdown
Member

@AryanSalmanpour AryanSalmanpour Dec 2, 2021

Choose a reason for hiding this comment

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

please put the for loop itself into the comment section as well. use /* */ to block the entire block instead of using // to comment on each line. so something like below:
// Uncomment to run test case with an xywhROI override

/* for (i = 0; i < images; i++)

...
...

}*/

Rpp32u kernelSize = additionalParam;
for (i = 0; i < images; i++)
{
// xywhROI override sample
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same comment as above

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

test_case_name = "erode";

Rpp32u kernelSize = additionalParam;
for (i = 0; i < images; i++)
Copy link
Copy Markdown
Member

@AryanSalmanpour AryanSalmanpour Dec 2, 2021

Choose a reason for hiding this comment

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

please put the for loop itself into the comment section as well. use /* */ to block the entire block instead of using // to comment on each line. so something like below:
// Uncomment to run test case with an xywhROI override

/* for (i = 0; i < images; i++)

...
...

}*/

@kiritigowda kiritigowda added the enhancement New feature or request label Dec 2, 2021
@r-abishek
Copy link
Copy Markdown
Member Author

@asalmanp Comments are more organized now - 9a1851b

Copy link
Copy Markdown
Member

@AryanSalmanpour AryanSalmanpour left a comment

Choose a reason for hiding this comment

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

@paveltc please verify this PR by running the RPP unit tests.

@paveltc
Copy link
Copy Markdown
Contributor

paveltc commented Dec 3, 2021

@asalmanp @kiritigowda @r-abishek This PR passes all of the unit tests.

@kiritigowda kiritigowda merged commit df2cab9 into ROCm:master Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants