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

Voxel Downsample for Tensor interface #6249

Merged
merged 13 commits into from Aug 11, 2023
Merged

Conversation

theNded
Copy link
Contributor

@theNded theNded commented Jul 6, 2023

This PR addresses #6172 #5519 and allows averaging properties in voxel downsample. Previously, points are simply put at grid corners.

To achieve this, the operation IndexAdd_ (similar for pytorch) for tensors is introduced.

An alternative would be reusing Benjamin's Voxel Pooling. We may revisit this later.

Now the behavior is the same as legacy, but there is a slight voxel coordinate offset due to implementation discrepancies. The legacy version computes a bound that slightly shifts the voxels.

Next TODOs

  • Further enhance backward compatibility via fixing discrepancies to legacy voxel downsample
  • More tests for IndexAdd_
  • Some screenshots

This change is Reviewable

@update-docs
Copy link

update-docs bot commented Jul 6, 2023

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

@theNded theNded marked this pull request as ready for review August 6, 2023 21:52
@theNded theNded requested a review from ssheorey August 6, 2023 22:00
Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @ssheorey, @theNded, and @yxlao)


cpp/pybind/t/geometry/pointcloud.cpp line 220 at r2 (raw file):

            "Downsamples a point cloud with a specified voxel size and a "
            "reduction type.",
            "voxel_size"_a, "reduction"_a = "mean");

Please update the documentation in google format and add a minimal example. See compute_convex_hull as an example.
Thank you!!! 😇

@theNded
Copy link
Contributor Author

theNded commented Aug 10, 2023

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @ssheorey, @theNded, and @yxlao)

cpp/pybind/t/geometry/pointcloud.cpp line 220 at r2 (raw file):

            "Downsamples a point cloud with a specified voxel size and a "
            "reduction type.",
            "voxel_size"_a, "reduction"_a = "mean");

Please update the documentation in google format and add a minimal example. See compute_convex_hull as an example. Thank you!!! innocent

Done. We could also use some time (and maybe GPT) to polish/fix the docs, including

  • Overloaded initializers;
  • Default parameters (esp. tensors);
  • Always follow param: Description and fix the incorrect param. Description.

Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

:lgtm: Thank you for the docstring fixes!

Reviewable status: 0 of 14 files reviewed, 1 unresolved discussion (waiting on @ssheorey and @yxlao)

@theNded theNded merged commit a9370f2 into isl-org:master Aug 11, 2023
36 of 37 checks passed
@theNded theNded deleted the t/voxeldownsample branch August 11, 2023 22:22
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.

None yet

2 participants