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

Test 4 directions earlier in skimage.feature.corner_fast #7394

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

spdfghi
Copy link
Contributor

@spdfghi spdfghi commented Apr 13, 2024

Closes #7388.

The correct implementation of Fast corner detection involves moving the fast check when n_fast >= 12 to the beginning of calculating the pixel values around the entire circle, which can significantly reduce the time spent on non-corner detection.

recreate of pr #7379

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

Test 4 directions earlier in `skimage.feature.corner_fast` which
should more than half the computation time for most cases.

@lagru
Copy link
Member

lagru commented Apr 15, 2024

Thank you! It may be some time until someone of us get's to review this, performance optimization aren't the highest priority right now. But it will get done eventually. 🙏

@mkcor
Copy link
Member

mkcor commented Apr 17, 2024

May I ask for a test or benchmark as a companion to this change? 🙏

@spdfghi
Copy link
Contributor Author

spdfghi commented Apr 17, 2024

May I ask for a test or benchmark as a companion to this change? 🙏
@mkcor in #7388 I made a simple test on a constant image and achieved results with over a fifty percent reduction in time. Essentially, this is equivalent to redundant measurements at non-corner positions. If you have any other thoughts, please let me know.
here is the code:

import numpy as np
import timeit
from skimage.feature import corner_fast, corner_peaks
square = np.zeros((1200, 1200))
square.astype(int)
def test():
    corner_fast(square)

execution_time = timeit.timeit(test, number=100)
print(execution_time)

@lagru
Copy link
Member

lagru commented Apr 24, 2024

Okay, using

import numpy as np
import skimage as ski
from skimage.feature import corner_fast

rng = np.random.default_rng(739473947394)

random = rng.random((1000, 1000))
zeros = np.zeros((1000, 1000))
hubble = ski.color.rgb2gray(ski.data.hubble_deep_field())

%timeit corner_fast(random, 12)
%timeit corner_fast(zeros, 12)
%timeit corner_fast(hubble, 12)

this optimization seems faster in two cases:

random zeros hubble
main 91 ms ± 35.2 µs 17.5 ms ± 13.6 µs 17 ms ± 17.4 µs
#7394 96.2 ms ± 75.6 5.85 ms ± 13.9 6.91 ms ± 5 µs

@lagru
Copy link
Member

lagru commented Apr 24, 2024

Updated the comment above. The optimization is slightly slower, for cases where there are a lot of potential corners. I suspect this is because ring_pixel = image[i + rp[k], j + cp[k]] and its comparisons are wasted. Maybe we can figure out an implementation that re-uses ring_pixel?

@spdfghi
Copy link
Contributor Author

spdfghi commented Apr 24, 2024

Updated the comment above. The optimization is slightly slower, for cases where there are a lot of potential corners. I suspect this is because ring_pixel = image[i + rp[k], j + cp[k]] and its comparisons are wasted. Maybe we can figure out an implementation that re-uses ring_pixel?

@lagru I believe it could be done by assigning these four variables first and then manually assigning the other eight, without using a for loop. However, is it necessary to do so? For the random images you are using, the number of feature points should be around 1/8 of the total number of image pixels, as determined by simple mathematical calculations. However, for real images, the number of feature points is generally less than 1/100. Additionally, for commonly used feature matching methods such as k-nearest neighbors (knn), the time complexity is O(N^2), which requires us to control the number of feature points. For an image of size 600*800, if the total number of feature points is around 1/8, the matching time will be on the order of 1e9, which is clearly far beyond the feature extraction time and is unacceptable.

@spdfghi
Copy link
Contributor Author

spdfghi commented Apr 24, 2024

@lagru Furthermore, I think rather than considering this pr as optimization, it's more accurate to say it's the correct implementation of the fast algorithm logic. The original idea of FAST, as presented in the paper, is to avoid computing 12 points by calculating 4 points, whereas the previous code failed to address this(and may cause confusion).

Copy link
Member

@mkcor mkcor left a comment

Choose a reason for hiding this comment

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

Nice! I also think the code reads better with this change.

@lagru
Copy link
Member

lagru commented Apr 30, 2024

In the paper it even says

Knowledge from the first 4 tests is discarded.

So all in all, I agree with @spdfghi. Let's not over-optimize what may very well be an edge case. This should be an improvement for most cases and also align more with the cited source.

@lagru lagru merged commit 0310a41 into scikit-image:main Apr 30, 2024
23 of 24 checks passed
@stefanv stefanv added this to the 0.24 milestone Apr 30, 2024
@lagru
Copy link
Member

lagru commented Apr 30, 2024

Thank you @spdfghi!

@lagru lagru changed the title improved speed of fast corner detect Test 4 directions earlier in skimage.feature.corner_fast Apr 30, 2024
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.

Investigate performance optimization in _corner_fast
4 participants