[sample_consensus] Fix and improve MLESAC #4575
Conversation
mvieth
added
changelog: enhancement
mvieth
force-pushed
the
mlesac
branch
3 times, most recently
from
7d6f950 to
dd2f0c4
Compare
mvieth
force-pushed
the
mlesac
branch
3 times, most recently
from
8ebb478 to
34147e1
Compare
common/include/pcl/common/common.h
Outdated
| * \ingroup common | ||
| */ | ||
| template<typename IteratorT, typename Functor> inline auto | ||
| computeMedian (IteratorT begin, IteratorT end, Functor f) noexcept -> typename std::iterator_traits<IteratorT>::value_type |
There was a problem hiding this comment.
noexcept(noexept(Functor)) might be better, but it doesn't matter much
|
@kunaltyagi Regarding the return type of |
|
That's why I commented about making the return type same as the return type of the lambda. The user will be able to switch it as needed. Choosing |
| if (size%2==0) | ||
| { // Even number of values | ||
| std::nth_element (begin, begin + (mid-1), end); | ||
| return (f(begin[mid-1]) + f(*(std::min_element (begin + mid, end)))) / 2.0; |
There was a problem hiding this comment.
I could also write static_cast<typename std::result_of<Functor(decltype(*begin))>::type>(2.0), but I don't see a case where there would be a difference between the three
There was a problem hiding this comment.
- /2.0 worst case:
int(4/2.0)is a double conversion followed by an int conversion - /2 worst case:
T(T()/2)involves just one conversion (unless T is char) - static_cast worst case: no conversion :) but readability is severely affected
TBH, This is a minor issue and I've already approved of the PR a while ago 😄 so I'm fine with it not being modified
Fixes bug: parentheses were wrong, was 2 * (sigma_ * sigma_) instead of (2 * sigma_ * sigma_). MLESAC works much better now Improves speed by precomputing factors
Improves speed of median by using nth_element and min_element instead of sort Use the new function in MLESAC and LMEDS
Please see commit messages.