Skip to content

Fix point-cloud limiting for float logits#29

Open
pentene wants to merge 1 commit into
mainfrom
dev/fix-pointcloud-limit
Open

Fix point-cloud limiting for float logits#29
pentene wants to merge 1 commit into
mainfrom
dev/fix-pointcloud-limit

Conversation

@pentene

@pentene pentene commented Jun 12, 2026

Copy link
Copy Markdown

Fixes point-cloud limiting so float logits are converted to boolean occupancy masks before as_points. Logits are still used for weighted sampling, but _limit now consistently returns Bool tensors.

@pentene pentene requested a review from thetianshuhuang June 12, 2026 22:07
@pentene pentene self-assigned this Jun 12, 2026
@pentene pentene added the bug Something isn't working label Jun 12, 2026
@pentene pentene linked an issue Jun 12, 2026 that may be closed by this pull request
@pentene pentene marked this pull request as ready for review June 12, 2026 22:12
@pentene pentene requested review from Copilot and removed request for Copilot June 12, 2026 22:12

@thetianshuhuang thetianshuhuang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there's a high level change that's been missed. This implementation is implicitly changing the abstraction; instead of arbitrary passing to PointCloudMetric.as_points and being turned into points, we're assuming grid-like is the argument, and as_points is responsible for taking an occupancy grid and turning it into N-dimensional Cartesian points. So if/when we add Chamfer for a modality that breaks this assumption (e.g., depth images), the current design won't work. This is in part also an existing issue with the code.

As an idea, what if we move max_points into Chamfer2D and Chamfer3D, and put _limit as a @staticmethod? Then Chamfer2D, Chamfer3D can "own" the responsibility of both subsampling and generating points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chamfer distance can OOM when there are less than 10k points

2 participants