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.

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

Make nan values detection case insensitive #1004

Merged
merged 2 commits into from
Nov 17, 2014
Merged

Make nan values detection case insensitive #1004

merged 2 commits into from
Nov 17, 2014

Conversation

VictorLamoine
Copy link
Contributor

NaN value detection was case sensitive, it is now case insensitive.
The boost header for this function is already included.
#1003

@taketwo
Copy link
Member

taketwo commented Nov 15, 2014

👍

@jspricke
Copy link
Member

We had this discussion back in the beginning of PCL and removed the feature of different nan styles, because it slowed down io quite a lot. That's why I wrote the pcl_pcd_convert_NaN_nan tool back then. I didn't test your implementation and afair the old one was different. Still we should check the speed implications of this change.

@VictorLamoine
Copy link
Contributor Author

PCL latest trunk compiled in Release mode
I used pcl_pcd2ply program (only the loading part) to check the loading time of pcd files.

The point cloud has been generated with MeshLab;
Filters > Create new mesh layer > Noisy isosurface > 256

The file contains no NaN values.

PCL latest trunk :

$ pcl_pcd2ply noisy_isocloud_ascii.pcd /dev/null.ply
> Loading noisy_isocloud_ascii.pcd [done, 11939 ms : 1768939 points]
> Loading noisy_isocloud_ascii.pcd [done, 12137 ms : 1768939 points]
> Loading noisy_isocloud_ascii.pcd [done, 12139 ms : 1768939 points]

With my pull request :

> Loading noisy_isocloud_ascii.pcd [done, 12219 ms : 1768939 points]
> Loading noisy_isocloud_ascii.pcd [done, 12121 ms : 1768939 points]
> Loading noisy_isocloud_ascii.pcd [done, 12231 ms : 1768939 points]

There is a 0.97% loss of performance from these results.

Then I used a simple program to changes values to nan in the point cloud. (see 2nd commit)

PCL latest trunk :

$ pcl_pcd2ply noisy_isocloudNaN_ascii.pcd /dev/null.ply
> Loading noisy_isocloudNaN_ascii.pcd [done, 13641 ms : 1768939 points]
> Loading noisy_isocloudNaN_ascii.pcd [done, 13709 ms : 1768939 points]
> Loading noisy_isocloudNaN_ascii.pcd [done, 13623 ms : 1768939 points]

With my pull request :

> Loading noisy_isocloud_nan_ascii.pcd [done, 13655 ms : 1768939 points]
> Loading noisy_isocloud_nan_ascii.pcd [done, 13769 ms : 1768939 points]
> Loading noisy_isocloud_nan_ascii.pcd [done, 14141 ms : 1768939 points]

There is a 1.42% loss of performance from these results.

I think the loss of performance is negligible.

jspricke added a commit that referenced this pull request Nov 17, 2014
Make nan values detection case insensitive
@jspricke jspricke merged commit fb4db10 into PointCloudLibrary:master Nov 17, 2014
@jspricke
Copy link
Member

Thanks a lot Victor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants