Skip to content

Conversation

@ubless607
Copy link

The colorize_bbox_3d function is expected to return an array of shape (N, 8, 3).
Since world_points_homo is shape (N, 4) and view_proj_mat is (4, 4), the matrix multiplication is required.

The `colorize_bbox_3d` function is expected to return an array of shape `(N, 8, 3)`. Since `world_points_homo` is shape `(N, 4)` and `view_proj_mat` is `(4, 4)`, the matrix multiplication is required.
@Copilot Copilot AI review requested due to automatic review settings October 11, 2025 17:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the world_to_image_pinhole function where incorrect matrix operations were producing wrong output shapes. The function is expected to return an array of shape (N, 8, 3) but was using th.dot instead of proper matrix multiplication.

Key Changes

  • Replaced th.dot with th.matmul for proper matrix multiplication between world_points_homo and view_proj_mat

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

view_proj_mat = view_mat @ proj_mat
world_points_homo = th.nn.functional.pad(world_points, (0, 1, 0, 0), value=1.0)
tf_points = th.dot(world_points_homo, view_proj_mat)
tf_points = th.matmul(world_points_homo, view_proj_mat)
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

The fix correctly addresses the shape issue. However, th.matmul with these shapes will produce (N, 4) output, not (N, 8, 3) as mentioned in the description. The actual shape depends on the input world_points dimensions. Consider verifying the expected input shape matches the desired output shape.

Copilot uses AI. Check for mistakes.
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.

1 participant