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

PERF: faster Hog computation #4957

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

WeiChungChang
Copy link
Contributor

@WeiChungChang WeiChungChang commented Sep 3, 2020

Current Hog takes use of 3 loops, ex: for 9 bins, 128 * 128 cells.

outer loop - go through each bins.
middle loop - go through each rows.
inner loop - go through each columns.

However, 2 loops should be beter:
outer loop - go through each rows.
inner loop - go through each columns.
then divide orientation by number_of_orientations_per_180 and put result into corresponding bin.

Ex, if number_of_orientations_per_180 is 20 (divide 180 degree by 9 bins so each bin's range is 20 degree);
if current pixel has orientation = 45, it locates at (45/20) = 2nd bin.
if current pixel has orientation = 18, it locates at (18/20) = 0th bin.
and so on.

The experiment shows this way can speed up the calculation about 10% ~ 20% of whole flow.
Since the execution time is dominated by (1) calculation bin distribution for each cell, and (2) normalization,
it implies for (1)only we speed up to 20~40%(may by more).

Here provided a test patch and attached a file to measure the difference.
It shows the difference is small enough to be ignored.

Roughly test with script attached shows the difference between original flow and refined flow is very small (use sum(abs(diff)) and for 4K image it is far smaller than 0.01).

The proposal patch keeps original flow, so a performance comparison can be executed by calling hog(....., fast=True) to select 2 loops method.

Please consider to rework current logic to provide better performance.

@sciunto sciunto changed the title refine Hog calculation PERF: faster Hog computation Sep 3, 2020
Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

@WeiChungChang thanks for this contribution! Apologies that you haven't received a response yet.

On first glance this is great! I'd like to request a few things:

  • you mentioned an attached script but I don't see one. Rather than a script, though, it would be great (actually necessary) to add a test function to skimage/feature/tests/test_hog.py that checks that the two paths are close for a range of example images.
  • you mention speed, it would so it would be great to see some benchmarks for this, again, both with fast and slow paths. See this section of the contributing guide for more!

After that I think we should be good — maybe some small tweaks to the code but nothing major.

Thank you again!

Comment on lines +162 to +165
if fast == 1:
print("use fast")
else:
print("use original")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if fast == 1:
print("use fast")
else:
print("use original")

Base automatically changed from master to main February 18, 2021 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants