Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup and improve unit test coverage for transformPointCloud functions #2245

Merged
merged 6 commits into from
Mar 7, 2018
Merged

Cleanup and improve unit test coverage for transformPointCloud functions #2245

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0862bcb
Minor clean-up in test_transforms.cpp
taketwo Mar 5, 2018
655276b
Move unit test for demeanPointCloud to test_centroid
taketwo Mar 5, 2018
993b04d
Move unit test for getTransFromUnitVectorsZY/XY to test_eigen
taketwo Mar 5, 2018
3b7c6d2
Split Matrix4Affine3Transform unit test
taketwo Mar 5, 2018
cc2c95b
Add comprehensive unit tests for transformPointCloud/WithNormals()
taketwo Mar 6, 2018
a24da4a
Use EXPECT_METADATA/XYZ_EQ assertions in centroid test
taketwo Mar 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions test/common/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
set(SUBSYS_NAME tests_common)
set(SUBSYS_DESC "Point cloud library common module unit tests")
PCL_SET_TEST_DEPENDENCIES(SUBSYS_DEPS common)
set(OPT_DEPS io features search kdtree octree)
set(OPT_DEPS io search kdtree octree)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️


set(DEFAULT ON)
set(build TRUE)
Expand All @@ -16,7 +16,7 @@ if (build)
PCL_ADD_TEST(common_common test_common FILES test_common.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_geometry test_geometry FILES test_geometry.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_copy_point test_copy_point FILES test_copy_point.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_centroid test_centroid FILES test_centroid.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_transforms test_transforms FILES test_transforms.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_int test_plane_intersection FILES test_plane_intersection.cpp LINK_WITH pcl_gtest pcl_common)
PCL_ADD_TEST(common_pca test_pca FILES test_pca.cpp LINK_WITH pcl_gtest pcl_common)
#PCL_ADD_TEST(common_spring test_spring FILES test_spring.cpp LINK_WITH pcl_gtest pcl_common)
Expand All @@ -32,10 +32,10 @@ if (build)

PCL_ADD_TEST(common_point_type_conversion test_common_point_type_conversion FILES test_point_type_conversion.cpp LINK_WITH pcl_gtest pcl_common)

if (BUILD_io AND BUILD_features)
PCL_ADD_TEST(a_transforms_test test_transforms
FILES test_transforms.cpp
if(BUILD_io)
PCL_ADD_TEST(common_centroid test_centroid
FILES test_centroid.cpp
LINK_WITH pcl_gtest pcl_io
ARGUMENTS "${PCL_SOURCE_DIR}/test/bun0.pcd")
endif (BUILD_io AND BUILD_features)
endif()
endif (build)
87 changes: 87 additions & 0 deletions test/common/test_centroid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,16 @@
#include <pcl/common/eigen.h>
#include <pcl/point_types.h>
#include <pcl/point_cloud.h>
#include <pcl/io/pcd_io.h>
#include <pcl/pcl_tests.h>

#include <pcl/common/centroid.h>

using namespace pcl;
using pcl::test::EXPECT_EQ_VECTORS;

pcl::PCLPointCloud2 cloud_blob;

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
TEST (PCL, compute3DCentroidFloat)
{
Expand Down Expand Up @@ -1009,9 +1012,93 @@ TEST (PCL, computeCentroid)
EXPECT_FLOAT_EQ (-500, centroid.curvature);
}

TEST (PCL, demeanPointCloud)
{
PointCloud<PointXYZ> cloud, cloud_demean;
fromPCLPointCloud2 (cloud_blob, cloud);

Eigen::Vector4f centroid;
compute3DCentroid (cloud, centroid);
EXPECT_NEAR (centroid[0], -0.0290809, 1e-4);
EXPECT_NEAR (centroid[1], 0.102653, 1e-4);
EXPECT_NEAR (centroid[2], 0.027302, 1e-4);
EXPECT_NEAR (centroid[3], 1, 1e-4);

// Check standard demean
demeanPointCloud (cloud, centroid, cloud_demean);
EXPECT_EQ (cloud_demean.is_dense, cloud.is_dense);
EXPECT_EQ (cloud_demean.width, cloud.width);
EXPECT_EQ (cloud_demean.height, cloud.height);
EXPECT_EQ (cloud_demean.size (), cloud.size ());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we expecting to happen to the previously set sensor origin and orientation?

EXPECT_NEAR (cloud_demean[0].x, 0.034503, 1e-4);
EXPECT_NEAR (cloud_demean[0].y, 0.010837, 1e-4);
EXPECT_NEAR (cloud_demean[0].z, 0.013447, 1e-4);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check that cloud_demean[0].w is preserved as 1, even after the demean.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, there is no .w member, only data[3]. Secondly, is it the part of the contract that this padding number is always 1.0? I'm not sure I ever seen this stated explicitly anywhere.

Copy link
Member

@SergioRAgostinho SergioRAgostinho Mar 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondly, is it the part of the contract that this padding number is always 1.0?

I makes sense to initialize by default with 1.0, but from then onwards, we can't assume anything anymore.

I'm not really sure how to handle this. In a sense, in most cases we're likely to have the homogeneous coordinate set to the same value in all points. But if it's not, I don't think demeanPointCloud is even handling it. So yeah, you've got a point.

EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].x, -0.048849, 1e-4);
EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].y, 0.072507, 1e-4);
EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].z, -0.071702, 1e-4);

std::vector<int> indices (cloud.size ());
for (int i = 0; i < static_cast<int> (indices.size ()); ++i) { indices[i] = i; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to try a case in which not all indices are used? Just the first and the last for instance?


// Check standard demean w/ indices
demeanPointCloud (cloud, indices, centroid, cloud_demean);
EXPECT_EQ (cloud_demean.is_dense, cloud.is_dense);
EXPECT_EQ (cloud_demean.width, cloud.width);
EXPECT_EQ (cloud_demean.height, cloud.height);
EXPECT_EQ (cloud_demean.size (), cloud.size ());

EXPECT_NEAR (cloud_demean[0].x, 0.034503, 1e-4);
EXPECT_NEAR (cloud_demean[0].y, 0.010837, 1e-4);
EXPECT_NEAR (cloud_demean[0].z, 0.013447, 1e-4);

EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].x, -0.048849, 1e-4);
EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].y, 0.072507, 1e-4);
EXPECT_NEAR (cloud_demean[cloud_demean.size () - 1].z, -0.071702, 1e-4);

// Check eigen demean
Eigen::MatrixXf mat_demean;
demeanPointCloud (cloud, centroid, mat_demean);
EXPECT_EQ (mat_demean.cols (), int (cloud.size ()));
EXPECT_EQ (mat_demean.rows (), 4);

EXPECT_NEAR (mat_demean (0, 0), 0.034503, 1e-4);
EXPECT_NEAR (mat_demean (1, 0), 0.010837, 1e-4);
EXPECT_NEAR (mat_demean (2, 0), 0.013447, 1e-4);

EXPECT_NEAR (mat_demean (0, cloud_demean.size () - 1), -0.048849, 1e-4);
EXPECT_NEAR (mat_demean (1, cloud_demean.size () - 1), 0.072507, 1e-4);
EXPECT_NEAR (mat_demean (2, cloud_demean.size () - 1), -0.071702, 1e-4);

// Check eigen demean + indices
demeanPointCloud (cloud, indices, centroid, mat_demean);
EXPECT_EQ (mat_demean.cols (), int (cloud.size ()));
EXPECT_EQ (mat_demean.rows (), 4);

EXPECT_NEAR (mat_demean (0, 0), 0.034503, 1e-4);
EXPECT_NEAR (mat_demean (1, 0), 0.010837, 1e-4);
EXPECT_NEAR (mat_demean (2, 0), 0.013447, 1e-4);

EXPECT_NEAR (mat_demean (0, cloud_demean.size () - 1), -0.048849, 1e-4);
EXPECT_NEAR (mat_demean (1, cloud_demean.size () - 1), 0.072507, 1e-4);
EXPECT_NEAR (mat_demean (2, cloud_demean.size () - 1), -0.071702, 1e-4);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider splitting these into 4 individual tests with a common fixture?

TEST_F (demeanPointCloud, PointCloud)
TEST_F (demeanPointCloud, PointCloudIndices)
TEST_F (demeanPointCloud, EigenMat)
TEST_F (demeanPointCloud, EigenMatIndices)


int
main (int argc, char** argv)
{
if (argc < 2)
{
std::cerr << "No test file given. Please download `bun0.pcd` and pass its path to the test." << std::endl;
return (-1);
}
if (io::loadPCDFile (argv[1], cloud_blob) < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to avoid having to load bunny from a PCD file. It would remove io as an optional dependency for these tests.

Why not populate randomly a 10x3 Eigen matrix and use that as a data source?

{
std::cerr << "Failed to read test file. Please download `bun0.pcd` and pass its path to the test." << std::endl;
return (-1);
}

testing::InitGoogleTest (&argc, argv);
return (RUN_ALL_TESTS ());
}
Expand Down
54 changes: 54 additions & 0 deletions test/common/test_eigen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,60 @@ TEST (PCL, transformBetween2CoordinateSystems)
EXPECT_LE ((transformationd.matrix())(i,j) - test(i,j), tolerance);
}

TEST (PCL, getTransFromUnitVectors)
{
Eigen::Vector3f xaxis (1, 0, 0), yaxis (0, 1, 0), zaxis (0, 0, 1);
Eigen::Affine3f trans;

trans = pcl::getTransFromUnitVectorsZY (zaxis, yaxis);
Eigen::Vector3f xaxistrans = trans * xaxis, yaxistrans = trans * yaxis, zaxistrans = trans * zaxis;
EXPECT_NEAR ((xaxistrans - xaxis).norm (), 0.0f, 1e-6);
EXPECT_NEAR ((yaxistrans - yaxis).norm (), 0.0f, 1e-6);
EXPECT_NEAR ((zaxistrans - zaxis).norm (), 0.0f, 1e-6);

trans = pcl::getTransFromUnitVectorsXY (xaxis, yaxis);
xaxistrans = trans * xaxis, yaxistrans = trans * yaxis, zaxistrans = trans * zaxis;
EXPECT_NEAR ((xaxistrans-xaxis).norm (), 0.0f, 1e-6);
EXPECT_NEAR ((yaxistrans-yaxis).norm (), 0.0f, 1e-6);
EXPECT_NEAR ((zaxistrans-zaxis).norm (), 0.0f, 1e-6);
}

TEST (PCL, getTransformation)
{
const float rot_x = 2.8827f;
const float rot_y = -0.31190f;
const float rot_z = -0.93058f;

Eigen::Affine3f affine;
pcl::getTransformation (0, 0, 0, rot_x, rot_y, rot_z, affine);

EXPECT_NEAR (affine (0, 0), 0.56854731f, 1e-4); EXPECT_NEAR (affine (0, 1), -0.82217032f, 1e-4); EXPECT_NEAR (affine (0, 2), -0.028107658f, 1e-4);
EXPECT_NEAR (affine (1, 0), -0.76327348f, 1e-4); EXPECT_NEAR (affine (1, 1), -0.51445758f, 1e-4); EXPECT_NEAR (affine (1, 2), -0.39082864f, 1e-4);
EXPECT_NEAR (affine (2, 0), 0.30686751f, 1e-4); EXPECT_NEAR (affine (2, 1), 0.24365838f, 1e-4); EXPECT_NEAR (affine (2, 2), -0.920034f, 1e-4);
}

TEST (PCL, getTranslationAndEulerAngles)
{
const float trans_x = 0.1f;
const float trans_y = 0.2f;
const float trans_z = 0.3f;
const float rot_x = 2.8827f;
const float rot_y = -0.31190f;
const float rot_z = -0.93058f;

Eigen::Affine3f affine;
pcl::getTransformation (trans_x, trans_y, trans_z, rot_x, rot_y, rot_z, affine);

float tx, ty, tz, rx, ry, rz;
pcl::getTranslationAndEulerAngles (affine, tx, ty, tz, rx, ry, rz);
EXPECT_NEAR (tx, trans_x, 1e-4);
EXPECT_NEAR (ty, trans_y, 1e-4);
EXPECT_NEAR (tz, trans_z, 1e-4);
EXPECT_NEAR (rx, rot_x, 1e-4);
EXPECT_NEAR (ry, rot_y, 1e-4);
EXPECT_NEAR (rz, rot_z, 1e-4);
}

/* ---[ */
int
main (int argc, char** argv)
Expand Down
Loading