search

PointCloudLibrary / pcl

Point Cloud Library (PCL)
https://pointclouds.org/
Other
9.84k stars 4.61k forks source link

pcl::PPFEstimation class uses PFH features instead of PPF features #1131

Open rosds opened 9 years ago

rosds commented 9 years ago

in the impl/ppf.hpp in the method to compute the pair features, PFH features are calculated instead of PPF. At line 77, the actual pcl::computePPFPairFeature method is commented and it is using pcl::computePairFeatures from pfh.hpp which calculate PFH features.

rosds commented 9 years ago

No it doesn't. You see, the PPF and PFH feature vectors are both composed of 4 floats, but their components represent different things. The PPFEstimation class should use the computePPFPairFeature defined in the src/ppf.hpp file, not the computePairFeatures defined in the src/pfh.hpp.

PPF: http://ar.in.tum.de/pub/drost2010CVPR/drost2010CVPR.pdf (section 3.1) PFH: http://ias.cs.tum.edu/_media/spezial/bib/rusu08icarcv.pdf (section II)

On Thu, Feb 5, 2015 at 11:23 AM, Anders Glent Buch <notifications@github.com

wrote:

I don't see a problem? The called function computePairFeatures() computes the exact same feature into the f1-f4 outputs.

— Reply to this email directly or view it on GitHub https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment-73024379 .

andersgb1 commented 9 years ago

Sorry, Alfonso, you are right. My bad.

I guess it would actually be interesting to see whether PPFRegistration performs best with the original f-values or the currently used PFH definitions... What do you think?

On Thu, Feb 5, 2015 at 12:03 PM, Alfonso Ros notifications@github.com wrote:

No it doesn't. You see, the PPF and PFH feature vectors are both composed of 4 floats, but their components represent different things. The PPFEstimation class should use the computePPFPairFeature defined in the src/ppf.hpp file, not the computePairFeatures defined in the src/pfh.hpp.

PPF: http://ar.in.tum.de/pub/drost2010CVPR/drost2010CVPR.pdf (section 3.1) PFH: http://ias.cs.tum.edu/_media/spezial/bib/rusu08icarcv.pdf (section II)

On Thu, Feb 5, 2015 at 11:23 AM, Anders Glent Buch < notifications@github.com

wrote:

I don't see a problem? The called function computePairFeatures() computes the exact same feature into the f1-f4 outputs.

— Reply to this email directly or view it on GitHub https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment- 73024379

.

— Reply to this email directly or view it on GitHub https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment-73029278 .

rosds commented 9 years ago

don't worry,

I guess that whoever made the change (because the computePPF method is commented) was exactly experimenting with that because in the PPFRegistration class the PFH features are also being used. The problem is that the change was not reverted (my guess) and now there is this problem with the features. I ran into this issue exactly because I was querying for PPF vectors to the PFH cloud obtained by the PPFEstimation.

I am currently querying for PFH vectors instead of PPF in order to not modify my PCL installation, the resulting matching pairs are more or less similar. I could not assert if one is better than the other based in what I am doing, but both features share similar ideas, so I would guess that both have similar performance regarding their power to represent the underlying structure of the cloud.

Nevertheless, I think this change must be reverted in order to restore the consistency with the classes.

On Thu, Feb 5, 2015 at 2:50 PM, Anders Glent Buch notifications@github.com wrote:

Sorry, Alfonso, you are right. My bad.

I guess it would actually be interesting to see whether PPFRegistration performs best with the original f-values or the currently used PFH definitions... What do you think?

On Thu, Feb 5, 2015 at 12:03 PM, Alfonso Ros notifications@github.com wrote:

No it doesn't. You see, the PPF and PFH feature vectors are both composed of 4 floats, but their components represent different things. The PPFEstimation class should use the computePPFPairFeature defined in the src/ppf.hpp file, not the computePairFeatures defined in the src/pfh.hpp.

PPF: http://ar.in.tum.de/pub/drost2010CVPR/drost2010CVPR.pdf (section 3.1) PFH: http://ias.cs.tum.edu/_media/spezial/bib/rusu08icarcv.pdf (section II)

On Thu, Feb 5, 2015 at 11:23 AM, Anders Glent Buch < notifications@github.com

wrote:

I don't see a problem? The called function computePairFeatures() computes the exact same feature into the f1-f4 outputs.

— Reply to this email directly or view it on GitHub https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment- 73024379

.

— Reply to this email directly or view it on GitHub < https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment-73029278

.

— Reply to this email directly or view it on GitHub https://github.com/PointCloudLibrary/pcl/issues/1131#issuecomment-73048530 .

Yumin-Sun-00 commented 6 years ago

Hey,

I am using the PPFEstimation.compute(), which seems like still an empty function. Does this means I should iteratively compute PPF for each point using pcl::comptePPFPairFeature(a list of args)?

Or should I use PFH?

stale[bot] commented 4 years ago

Marking this as stale due to 30 days of inactivity. It will be closed in 7 days if no further activity occurs.