cartographer-project / point_cloud_viewer

View billions of points in your browser.
Apache License 2.0
337 stars 98 forks source link

Improve testing by sorting by idx. #368

Closed feuerste closed 4 years ago

nnmm commented 4 years ago

That only works for 2^24 points, right? Could we have an assertion somewhere that we have no more than that number?

feuerste commented 4 years ago

It should even work for more than that, because I still use the old sorting logic in addition, so if we have multiple points with the same index, they will next be sorted by position.

nnmm commented 4 years ago

But the index will wrap around, and then point 2^24 + 1 will come after 0, right?

nnmm commented 4 years ago

For slightly-fuzzy equality checking, we'd also need to do explicit iteration through both vectors instead of zipping, right? I.e. if the indices of the current elements don't match, skip the element with the smaller index (and count how often we do that).

feuerste commented 4 years ago

But the index will wrap around, and then point 2^24 + 1 will come after 0, right?

Yes, but it will be the same for both octree points and s2 points, so we will only run into problems after sorting, if points with common indices are again very close to each other.

feuerste commented 4 years ago

For slightly-fuzzy equality checking, we'd also need to do explicit iteration through both vectors instead of zipping, right? I.e. if the indices of the current elements don't match, skip the element with the smaller index (and count how often we do that).

Yes, but since I found a seed which works now, I thought we could do this in another PR.

feuerste commented 4 years ago

Thanks for the review!