Summary
#185 covers the missing refit in ransac(). lmeds() has the identical gap, and it was explicitly put out of scope there. Filing it so the two don't drift apart.
lmeds() keeps the hypothesis with the lowest median squared error, then derives a robust threshold and classifies inliers:
if (result) {
sigma = 2.5 * 1.4826 * (1 + 5.0 / (count - model_points)) * Math.sqrt(min_median);
sigma = Math.max(sigma, 0.001);
numinliers = this.find_inliers(kernel, model, from, to, count, sigma, err, curr_mask.data);
if (mask) curr_mask.copy_to(mask);
result = numinliers >= model_points;
}
It computes the inlier set — and then never uses it. model still holds the winning minimal-sample fit.
Its doc comment makes the same false promise ransac()'s does:
"...then derives an inlier threshold from the winning median's robust standard deviation and refits on the inliers."
OpenCV
Same story as #185: LMeDSPointSetRegistrator::run in ptsetreg.cpp does not refit either. The refit is in findHomography, and the branch covers both methods:
if( method == RANSAC || method == LMEDS )
cb->runKernel( src, dst, H );
Mat H8(9, 1, CV_64F, H.ptr<double>());
LMSolver::create(makePtr<HomographyRefineCallback>(src, dst), 10)->run(H8);
So whichever option #185 lands on, LMEDS goes through the same path. Fixing only ransac() would leave jsfeatNext with two robust estimators that behave differently for no reason a user could predict.
Note in LMEDS's favour
lmeds() is actually better placed than ransac() for this: it already has the correct inlier mask sitting in curr_mask at the point where the refit belongs, because it classifies once at the end rather than once per iteration. It does not have ransac()'s stale-curr_mask problem, so no best_mask buffer is needed here.
Proposed change
Inside the existing if (result) block, after find_inliers:
if (numinliers >= model_points) {
const in0: point_t[] = [], in1: point_t[] = [];
for (let i = 0; i < count; ++i) {
if (curr_mask.data[i]) { in0.push(from[i]); in1.push(to[i]); }
}
if (in0.length > model_points && kernel.run(in0, in1, M, in0.length) > 0) {
M.copy_to(model);
numinliers = this.find_inliers(kernel, model, from, to, count, sigma, err, curr_mask.data);
}
}
if (mask) curr_mask.copy_to(mask);
result = numinliers >= model_points;
Note the mask is recomputed against the refit model before being handed back, so mask and model describe the same transform (OpenCV step (c) in #185).
Under Option B of #185 this moves into the composed find_homography() entry point instead, with lmeds left at jsfeat parity.
Acceptance criteria
Related
Summary
#185 covers the missing refit in
ransac().lmeds()has the identical gap, and it was explicitly put out of scope there. Filing it so the two don't drift apart.lmeds()keeps the hypothesis with the lowest median squared error, then derives a robust threshold and classifies inliers:It computes the inlier set — and then never uses it.
modelstill holds the winning minimal-sample fit.Its doc comment makes the same false promise
ransac()'s does:OpenCV
Same story as #185:
LMeDSPointSetRegistrator::runinptsetreg.cppdoes not refit either. The refit is infindHomography, and the branch covers both methods:So whichever option #185 lands on, LMEDS goes through the same path. Fixing only
ransac()would leave jsfeatNext with two robust estimators that behave differently for no reason a user could predict.Note in LMEDS's favour
lmeds()is actually better placed thanransac()for this: it already has the correct inlier mask sitting incurr_maskat the point where the refit belongs, because it classifies once at the end rather than once per iteration. It does not haveransac()'s stale-curr_maskproblem, so nobest_maskbuffer is needed here.Proposed change
Inside the existing
if (result)block, afterfind_inliers:Note the mask is recomputed against the refit model before being handed back, so
maskandmodeldescribe the same transform (OpenCV step (c) in #185).Under Option B of #185 this moves into the composed
find_homography()entry point instead, withlmedsleft at jsfeat parity.Acceptance criteria
motion_estimator.ransac: doc comment promises a refit over all inliers; jsfeatNext has no layer that performs one #185 (A or B) applied consistently tolmedskernel.run() <= 0fallbacklmeds's doc comment and code agreetests/parity/motion_estimator.test.ts's lmeds case handled the same way as the ransac case (reclassified under A, untouched under B)Related
motion_estimator.ransac: doc comment promises a refit over all inliers; jsfeatNext has no layer that performs one #185