[sfm] Performance improvements: optimize RemoveOutliers_AngleError

[0xfaded]

• Use greedy hueristic to avoid O(N^2) code in vast majority of cases
• Use Eigen matrix operations to speed up O(N^2) operations
• Add omp parallel

Description

This diff removes an N^2 loop which becomes relevant when processing large numbers of images. On my setup, when processing 8000 images 50% of the time is spent in this routine.

Features list

Implementation remarks

The loop in question searches for the largest angle between view directions. In my implementation, I first attempt a simple greedy algorithm which in almost all cases exits in sub-linear time if the angle exists. If the greedy algorithm fails, I revert to a n^2 implemented in Eigen for speed..

I haven't done a thorough analysis, but I suspect the overall expected runtime is << linear.

[fabiencastan]

Hi @0xfaded,
Thanks for the PR. I haven't looked at your implementation yet, but it sounds like a great idea.
Could you provide some figures on the impact of the proposed changes on performance?

[0xfaded]

Hi Fabien.

For large datasets, in my case over 8000 images, this reduces the SFM time from 1h40 to 50 minutes on my machine.

This check is run on every feature after every bundle adjustment, so for datasets with large tracks this routine actually becomes the bottleneck.

In terms of the actual implementation, every effort is made to exit the loop as early as possible. In most cases, this reduces the expexted complexity from O(N^2) to near O(1).

For small datasets, this will not be noticeable. Is there a particular benchmark you want me to run?

[fabiencastan]

Wow that sounds amazing!
Yes, we are limiting the number of cameras that we are considering in the BA with the "local BA" strategy, but at the same time we are doing this test on all landmarks at each step...

[fabiencastan]

When we use the local BA, we may retrieve the cameras that have been optimized in the BA (local BA state per pose) and for each camera we have a pre-computed list of tracks id visible in this camera (_map_tracksPerView).
So we could be able to test only the landmarks that could have change in the BA.

[0xfaded]

When we use the local BA, we may retrieve the cameras that have been optimized in the BA (local BA state per pose) and for each camera we have a pre-computed list of tracks id visible in this camera (_map_tracksPerView).
So we could be able to test only the landmarks that could have change in the BA.

Thanks @fabiencastan for the suggestion. However I'd prefer to implement this as a separate pull request.

[simogasp]

Thanks for the nice contribution!
I just left some minor syntactical fixes to be more uniform to the code style

[fabiencastan]

CI error on windows:

C:\projects\alicevision\src\aliceVision\sfm\sfmFilters.cpp(77,1): error C3015: initialization in OpenMP 'for' statement has improper form [

fixed

[fabiencastan]

@0xfaded Thanks for the nice work!