pcl::isFinite type implementation #2664
Comments
|
I am very close. Here is my current code snippet (I am focusing on registered points at the moment). I am having a hard time with pcl::detail::FiniteMapper::operator () (). It seems that boost mpl is having difficulties in unwrapping the iterator for the point mpl vector. I have tried an alternate method using boost::mpl::for_each with the same trouble #include <pcl/point_types.h>
#include <pcl/point_traits.h>
#include <pcl/for_each_type.h>
#include <type_traits>
namespace pcl
{
namespace detail
{
/** maps values of point to finite bool */
template<typename PointT>
struct FiniteMapper
{
FiniteMapper(const PointT& pt, bool& isInfinite_)
: pt(pt), isInfinite_(isInfinite_)
{
}
template<typename Key> inline void
operator () ()
{
typedef typename pcl::traits::datatype<PointT, Key>::type keyType;
keyType checkValue;
memcpy(reinterpret_cast<uint8_t*>(&checkValue),
reinterpret_cast<const uint8_t*>(&pt) + pcl::traits::offset<PointT, Key>::value,
sizeof(keyType));
isInfinite_ |= std::isfinite(checkValue);
}
const PointT& pt;
bool& isInfinite_;
};
}
/** Tests if a value is finite
* param[in] val class to check if finite
* return true if finite, false otherwise
*/
template<typename PointT>
inline bool isFinite(const PointT& pt)
{
typedef typename pcl::traits::fieldList<PointT>::type pointList;
bool isInfinite = false;
detail::FiniteMapper<PointT> mapper(pt, isInfinite);
for_each_type <pointList>(mapper);
return !isInfinite;
}
}Maintainer Edit: sintax highlight |
SergioRAgostinho
added
module: common
changelog: enhancement
|
I modified your snippet and this is working for me now. #include <pcl/point_types.h>
#include <pcl/point_traits.h>
#include <pcl/for_each_type.h>
namespace pcl
{
namespace traits
{
template<typename PointT>
struct IsFinite
{
IsFinite (const PointT& pt, bool& value) : pt (pt), value (value)
{
typedef typename traits::fieldList<PointT>::type FieldList;
for_each_type <FieldList>(*this);
}
template<typename Key> inline void
operator () ()
{
typedef typename datatype<PointT, Key>::type KeyType;
const KeyType* value = (const KeyType*)((const char*)&pt + offset<PointT, Key>::value);
this->value &= std::isfinite(*value);
}
const PointT& pt;
bool& value;
};
}
/** Tests if a value is finite
* param[in] val class to check if finite
* return true if finite, false otherwise
*/
template<typename PointT>
bool newIsFinite(const PointT& pt)
{
bool is_finite = true;
traits::IsFinite<PointT> mapper (pt, is_finite);
return is_finite;
}
}
int
main()
{
pcl::PointXYZRGBNormal p;
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
p.normal_z = std::numeric_limits<float>::infinity();
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
p.normal_z = 0;
p.y = std::numeric_limits<float>::quiet_NaN();
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
return 0;
} |
|
I still get the same compilation error that the call to fpclassify is ambiguous. What is your environment? I am using MSVC 15.9.3 on Windows 10. I am wondering how I am having a problem with this given that copy_point.hpp uses the same mechanism. I have attached the full output trace in case anyone recognizes this funny business. |
|
My env is Given the error, the fix here will likely be explicitly invoking the some function with the correct template instantiation i.e. function<KeyType>()I am just confused because the log is pointing to a couple of lines which in my version of the lib do not make sense. Specifically I'm now trying to compile the full library replacing the original definitions with this new version. It worked ok. #pragma once
#ifdef _MSC_VER
#include <Eigen/src/StlSupport/details.h>
#endif
#include <pcl/for_each_type.h>
namespace pcl
{
// /** Tests if the 3D components of a point are all finite
// * param[in] pt point to be tested
// * return true if finite, false otherwise
// */
// template <typename PointT> inline bool
// isFinite (const PointT &pt)
// {
// return (pcl_isfinite (pt.x) && pcl_isfinite (pt.y) && pcl_isfinite (pt.z));
// }
// #ifdef _MSC_VER
// template <typename PointT> inline bool
// isFinite (const Eigen::internal::workaround_msvc_stl_support<PointT> &pt)
// {
// return isFinite<PointT> (static_cast<const PointT&> (pt));
// }
// #endif
// template<> inline bool isFinite<pcl::RGB> (const pcl::RGB&) { return (true); }
// template<> inline bool isFinite<pcl::Label> (const pcl::Label&) { return (true); }
// template<> inline bool isFinite<pcl::Axis> (const pcl::Axis&) { return (true); }
// template<> inline bool isFinite<pcl::Intensity> (const pcl::Intensity&) { return (true); }
// template<> inline bool isFinite<pcl::MomentInvariants> (const pcl::MomentInvariants&) { return (true); }
// template<> inline bool isFinite<pcl::PrincipalRadiiRSD> (const pcl::PrincipalRadiiRSD&) { return (true); }
// template<> inline bool isFinite<pcl::Boundary> (const pcl::Boundary&) { return (true); }
// template<> inline bool isFinite<pcl::PrincipalCurvatures> (const pcl::PrincipalCurvatures&) { return (true); }
// template<> inline bool isFinite<pcl::SHOT352> (const pcl::SHOT352&) { return (true); }
// template<> inline bool isFinite<pcl::SHOT1344> (const pcl::SHOT1344&) { return (true); }
// template<> inline bool isFinite<pcl::ReferenceFrame> (const pcl::ReferenceFrame&) { return (true); }
// template<> inline bool isFinite<pcl::ShapeContext1980> (const pcl::ShapeContext1980&) { return (true); }
// template<> inline bool isFinite<pcl::UniqueShapeContext1960> (const pcl::UniqueShapeContext1960&) { return (true); }
// template<> inline bool isFinite<pcl::PFHSignature125> (const pcl::PFHSignature125&) { return (true); }
// template<> inline bool isFinite<pcl::PFHRGBSignature250> (const pcl::PFHRGBSignature250&) { return (true); }
// template<> inline bool isFinite<pcl::PPFSignature> (const pcl::PPFSignature&) { return (true); }
// template<> inline bool isFinite<pcl::PPFRGBSignature> (const pcl::PPFRGBSignature&) { return (true); }
// template<> inline bool isFinite<pcl::NormalBasedSignature12> (const pcl::NormalBasedSignature12&) { return (true); }
// template<> inline bool isFinite<pcl::FPFHSignature33> (const pcl::FPFHSignature33&) { return (true); }
// template<> inline bool isFinite<pcl::VFHSignature308> (const pcl::VFHSignature308&) { return (true); }
// template<> inline bool isFinite<pcl::ESFSignature640> (const pcl::ESFSignature640&) { return (true); }
// template<> inline bool isFinite<pcl::IntensityGradient> (const pcl::IntensityGradient&) { return (true); }
// template<> inline bool isFinite<pcl::BRISKSignature512> (const pcl::BRISKSignature512&) { return (true); }
// // specification for pcl::PointXY
// template <> inline bool
// isFinite<pcl::PointXY> (const pcl::PointXY &p)
// {
// return (pcl_isfinite (p.x) && pcl_isfinite (p.y));
// }
// // specification for pcl::BorderDescription
// template <> inline bool
// isFinite<pcl::BorderDescription> (const pcl::BorderDescription &p)
// {
// return (pcl_isfinite (p.x) && pcl_isfinite (p.y));
// }
// // specification for pcl::Normal
// template <> inline bool
// isFinite<pcl::Normal> (const pcl::Normal &n)
// {
// return (pcl_isfinite (n.normal_x) && pcl_isfinite (n.normal_y) && pcl_isfinite (n.normal_z));
// }
namespace traits
{
template<typename PointT>
struct IsFinite
{
IsFinite (const PointT& pt, bool& value) : pt (pt), value (value)
{
typedef typename traits::fieldList<PointT>::type FieldList;
for_each_type <FieldList>(*this);
}
template<typename Key> inline void
operator () ()
{
typedef typename datatype<PointT, Key>::type KeyType;
const KeyType* value = (const KeyType*)((const char*)&pt + offset<PointT, Key>::value);
this->value &= pcl_isfinite(*value);
}
const PointT& pt;
bool& value;
};
}
/** Tests if a value is finite
* param[in] val class to check if finite
* return true if finite, false otherwise
*/
template<typename PointT>
bool isFinite(const PointT& pt)
{
bool is_finite = true;
traits::IsFinite<PointT> mapper (pt, is_finite);
return is_finite;
}
} |
|
@SergioRAgostinho your snippet looks reasonable to me, I was able to mentally compile it without errors :) I think you should not comment out template specializations and the |
I need to get that upgrade myself next time I go to maintenance. This snippet just get's the basics done.
Agreed. Ideally I would like to completely ignore non floating point fields, to compose a |
|
That's a good idea, then we won't need explicit instantiations and it will work for custom point types out of the box. |
|
Second attempt #include <pcl/point_types.h>
#include <pcl/point_traits.h>
#include <pcl/for_each_type.h>
namespace pcl
{
namespace traits
{
template<typename PointT>
struct IsFinite
{
IsFinite (const PointT& pt, bool& value) : pt (pt), value (value)
{
typedef typename traits::fieldList<PointT>::type FieldList;
for_each_type<FieldList>(*this);
}
template<typename Key> inline
typename boost::enable_if_c<boost::is_float<typename datatype<PointT, Key>::type>::value>::type
operator () ()
{
std::cout << "iter " << typeid(typename asType<datatype<PointT, Key>::value >::type).name() << std::endl;
typedef typename datatype<PointT, Key>::type KeyType;
const KeyType* value = (const KeyType*)((const char*)&pt + offset<PointT, Key>::value);
this->value &= std::isfinite(*value);
}
template<typename Key> inline
typename boost::disable_if_c<boost::is_float<typename datatype<PointT, Key>::type>::value>::type
operator () () {}
const PointT& pt;
bool& value;
};
}
/** Tests if a value is finite
* param[in] val class to check if finite
* return true if finite, false otherwise
*/
template<typename PointT>
bool newIsFinite(const PointT& pt)
{
bool is_finite = true;
traits::IsFinite<PointT> mapper (pt, is_finite);
return is_finite;
}
}
int
main()
{
pcl::PointXYZRGBA p;
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
p.z = std::numeric_limits<float>::infinity();
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
p.z = 0;
p.y = std::numeric_limits<float>::quiet_NaN();
std::cout << p << '\n' << pcl::newIsFinite (p) << std::endl;
return 0;
}which outputs This get's the job done but there's still that extra call to the empty function, which I assume is optimized out. The hurdle here is that this Edit: I just noticed all our histogram types are float arrays :') Basically we cannot get rid of the specializations. |
|
We can always check the value of |
|
The current proposal does not contemplate arrays, that's true. But my comment was not so much on the fact that they're arrays, but that they are histograms whose elements are floats instead of an integer type. Their specialization of |
|
We can do whatever we want: iterate and check whether each value in the histogram is finite, or immediately output true. But anyway, I think we have a more conceptual question to answer here. As it is, Most (all?) algorithms that use this function actually mean to test only x, y, z. If we were to adopt the method we are discussing, this would make a big difference for them. All of a sudden, points with finite coordinates but, say, invalid curvature will be skipped. Besides, the runtime for points that have many With this in mind, I'd certainly avoid replacing |
|
You're right. Now I feel like I went down the rabbit whole too quickly on this one. :) Oh well was still an interesting crash course into MPL. By the way... |
|
I've never got around to use this one. Once I even started to read intro/tutorial, but then realized that required standard and compiler versions are too high for my project and I can not lift them due to CUDA issues. |
|
Kicking this conversation (since it was referred) alive. My rationale for a function is not having to repeat those checks again.
Agreed. How about simpler and more narrow focussed functions like Will be easier to write these functions too Pro: less typing in code, not dependent on new future types in PCL EDIT: struct Point {
int x,y,z;
int normal_x, normal_y, normal_z;
};
template <class PointT>
using HasNormal = typename std::enable_if<traits::has_normal<PointT>::value, bool>::type;
template <class PointT, HasNormal<PointT> = true>
constexpr bool isNormalFinite(const PointT& point) noexcept
{
return point.normal_x == point.normal_y; // could even use MPL trick to compare
}
constexpr auto val = isNormalFinite(Point{}); |
|
Please have a look at the pcl/common/include/pcl/point_types.h Line 754 in 96b6f4e The meta-functions from this namespace can be used to test the availability of particular fields in a point. |
Brigand might be the library for you |
|
Thanks for the link, haven't heard of it. But too late anyway :) |
The downside is that
pcl::isFiniteis not actually implemented for a couple of types. It requires a type-trait/meta-programming magic, in order to avoid having to define the method explicitly to all newly added types.Originally posted by @SergioRAgostinho in #2518 (comment)
The text was updated successfully, but these errors were encountered: