FarthestPointDownSample: arg to specify start index to start downsampling from

[abhishek47kashyap]

Closes #7049.

The function PointCloud::FarthestPointDownSample() is overloaded to accept a start_index in addition to the existing num_samples.

Type

• Bug fix (non-breaking change which fixes an issue): Fixes #
• New feature (non-breaking change which adds functionality). Resolves #
• Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

See ticket linked above.

Checklist:

• I have run python util/check_style.py --apply to apply Open3D code style
  to my code.
• This PR changes Open3D behavior or adds new functionality.
  • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
    updated accordingly.
  • I have added or updated C++ and / or Python unit tests OR included test
    results (e.g. screenshots or numbers) here.
• I will follow up and update the code if CI fails.
• For fork PRs, I have selected Allow edits from maintainers.

Description

This PR overloads the existing PointCloud::FarthestPointDownSample() to accept size_t start_index as the desired index to start farthest point downsampling from. Both geometry and t/geometry have been updated and checks added to ensure start_index is valid. Input arguments num_samples and start_index are marked const (existing num_samples was previously not const).

Python bindings have been updated with py::overload_cast. Docstrings and unit tests have been modified accordingly.

Output of unit tests

~/open3d/build$ ./bin/tests --gtest_filter=PointCloud.FarthestPointDownSample*
[Open3D INFO] CompilerInfo: C++ 17, GNU 11.4.0, CUDA disabled.
[Open3D INFO] CPUInfo: 8 cores, 16 threads.
[Open3D INFO] ISAInfo: AVX512SKX instruction set used in ISPC code.
Note: Google Test filter = PointCloud.FarthestPointDownSample*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from PointCloud
[ RUN      ] PointCloud.FarthestPointDownSample
[Open3D DEBUG] Pointcloud down sampled from 8 points to 4 points.
[Open3D DEBUG] Pointcloud down sampled from 8 points to 4 points.
[       OK ] PointCloud.FarthestPointDownSample (0 ms)
[----------] 1 test from PointCloud (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 1 test.



~/open3d/build$ ./bin/tests --gtest_filter=PointCloud/PointCloudPermuteDevices.FarthestPointDownSample*
[Open3D INFO] CompilerInfo: C++ 17, GNU 11.4.0, CUDA disabled.
[Open3D INFO] CPUInfo: 8 cores, 16 threads.
[Open3D INFO] ISAInfo: AVX512SKX instruction set used in ISPC code.
Note: Google Test filter = PointCloud/PointCloudPermuteDevices.FarthestPointDownSample*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from PointCloud/PointCloudPermuteDevices
[ RUN      ] PointCloud/PointCloudPermuteDevices.FarthestPointDownSample/0
[Open3D DEBUG] Pointcloud down sampled from 8 points to 4 points.
[Open3D DEBUG] Pointcloud down sampled from 8 points to 4 points.
[       OK ] PointCloud/PointCloudPermuteDevices.FarthestPointDownSample/0 (2 ms)
[----------] 1 test from PointCloud/PointCloudPermuteDevices (2 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (2 ms total)
[  PASSED  ] 1 test.

[update-docs]

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

[abhishek47kashyap]

This check ensures that the static_cast down below does not result in undefined behavior. Is there a better way to do this check?

Also, not fully sure why int64_t is being used in the first place in this function

[abhishek47kashyap]

Another test could be added with a non-zero start_index if I have a way to know what expected value to check against

[benjaminum]

You can compute the result for non-zero start_index with the function and then use this in the unit test. I think this is ok here because there were no major changes to the algorithm.

[benjaminum]

Thanks @abhishek47kashyap for the PR! Please have a look at the comments

[benjaminum]

You can compute the result for non-zero start_index with the function and then use this in the unit test. I think this is ok here because there were no major changes to the algorithm.