borglab
Add report from matcher with correct inlier ratio computation
This actually computes inlier ratio w.r.t. gt model with # putatives as the denominator.
- Add MatcherReport class and .evaluate() method of base matcher class
Solves https://github.com/borglab/gtsfm/issues/306
Adds section:
I'm not sure I really agree with any of these changes. I think spreading the evaluation tasks around to all the different modules just increases complexity. I think more responsibility should be given to the TwoViewEstimationReport (or other derived subclasses).
Moreover, we are now passing around three different reports (two of which are just copies of each other) that could easily be storing the same information.
How does this solve #306?
306 notes that the "inlier ratio w.r.t. GT model" was far higher than "inlier ratio w.r.t. est model". We would actually expect the following two fractions:
- Fraction 1: (# verified correspondences w.r.t. est model ) / (# putatives )
- Fraction 2: (# verified correspondences w.r.t. GT model ) / (# putatives )
to be very close to each other.
It turns out the "inlier ratio w.r.t. GT model" that we were reporting in the Verifier's output was actually
- Fraction 3: (# verified correspondences that are verified by GT model) / (# verified correspondences).
The denominator was different, explaining the issue we saw in 306. This PR provides the computation of Fraction 2 and adds it to the report.
How does this solve #306?
306 notes that the "inlier ratio w.r.t. GT model" was far higher than "inlier ratio w.r.t. est model". We would actually expect the following two fractions:
- Fraction 1: (# verified correspondences w.r.t. est model ) / (# putatives )
- Fraction 2: (# verified correspondences w.r.t. GT model ) / (# putatives )
to be very close to each other.
It turns out the "inlier ratio w.r.t. GT model" that we were reporting in the Verifier's output was actually
- Fraction 3: (# verified correspondences that are verified by GT model) / (# verified correspondences).
The denominator was different, explaining the issue we saw in 306. This PR provides the computation of Fraction 2 and adds it to the report.
@akshay-krishnan @ayushbaid @travisdriver wanted to ping you guys on this again
Could you pull from master and update the PR? Or would you like one of us to "take it from here"?
Everything in the PR looks good, except for the custom matcher report. This adds more boilerplate. We created the metrics struct to avoid such custom reports per module and to share operations across metrics. We could either return (num_inliers, inlier_ratio) as tuples (simpler), or return a GtsfmMetricsGroup and merge groups from different matchers later.
