Skip to content

Fix robot forward kinematics#223

Merged
matafela merged 3 commits into
mainfrom
cj/fix-robot-fk-diff-device
Apr 8, 2026
Merged

Fix robot forward kinematics#223
matafela merged 3 commits into
mainfrom
cj/fix-robot-fk-diff-device

Conversation

@matafela
Copy link
Copy Markdown
Collaborator

@matafela matafela commented Apr 8, 2026

Description

Fix robot forward kinematic error when qpos has different device with robot.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Dependencies have been updated, if applicable.

Copilot AI review requested due to automatic review settings April 8, 2026 04:19
Copy link
Copy Markdown
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

Fixes forward kinematics failures when qpos is on a different torch device than the Robot/solver, by moving qpos onto self.device before invoking FK.

Changes:

  • In compute_fk, move qpos to the robot device prior to calling solver.get_fk.
  • In compute_batch_fk, move qpos to the robot device prior to reshaping and calling solver.get_fk.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread embodichain/lab/sim/objects/robot.py Outdated
Comment on lines +488 to +504
if qpos.device != self.device:
qpos_ = qpos.to(self.device)
else:
qpos_ = qpos
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

compute_fk advertises qpos can be a torch.Tensor | np.ndarray | None, but this code path assumes a Tensor (dim(), unsqueeze(), and now device). Consider normalizing qpos at the start with the existing to_tensor(..., device=self.device) helper (and/or narrowing the type annotation/docstring) so numpy inputs are either supported as documented or rejected explicitly with a clear error.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can directly use to (device). This interface will check the device internally.

@@ -633,8 +638,13 @@ def compute_batch_fk(
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[1]}."
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

In compute_batch_fk, the shape check uses qpos.shape[2] != solver.dof but the error message reports qpos.shape[1]. This will misreport the number of joints; it should report the last dimension (shape[2]) to match the validation.

Suggested change
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[1]}."
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[2]}."

Copilot uses AI. Check for mistakes.
Comment thread embodichain/lab/sim/objects/robot.py Outdated
Comment on lines +488 to +504
if qpos.device != self.device:
qpos_ = qpos.to(self.device)
else:
qpos_ = qpos
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can directly use to (device). This interface will check the device internally.

chenjian added 2 commits April 8, 2026 14:39
Copilot AI review requested due to automatic review settings April 8, 2026 06:41
Copy link
Copy Markdown
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -633,8 +634,9 @@ def compute_batch_fk(
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[1]}."
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

In compute_batch_fk, the dof check compares qpos.shape[2] to solver.dof, but the error message reports got {qpos.shape[1]} (batch dimension) instead of the joint dimension. This can be misleading when debugging shape issues; the message should report qpos.shape[2] (or the actual joint-axis size being validated).

Suggested change
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[1]}."
f"Joint positions shape mismatch. Expected {solver.dof} joints, got {qpos.shape[2]}."

Copilot uses AI. Check for mistakes.
@matafela matafela merged commit 567a16e into main Apr 8, 2026
8 checks passed
@matafela matafela deleted the cj/fix-robot-fk-diff-device branch April 8, 2026 07:43
yuecideng pushed a commit that referenced this pull request Apr 10, 2026
Co-authored-by: chenjian <chenjian@dexforce.com>
chase6305 pushed a commit that referenced this pull request Apr 16, 2026
Co-authored-by: chenjian <chenjian@dexforce.com>
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.

3 participants