Skip to content

run_inference discards the re-run forward's joint_global_rots, so pred rotations are pre-hand-refinement #139

Description

@todderasesareddot

Summary

In run_inference, after the hand decoder's pose is fused into the MHR params, the model re-runs the MHR forward to refresh vertices / keypoints / joint coords. That re-run also returns joint_global_rots — but it is never stored, so pose_output["mhr"]["joint_global_rots"] keeps the pre-refinement value from the body decoder.

The result is an inconsistent output bundle: hand_pose_params, pred_vertices and pred_joint_coords carry the refined hand articulation, while the joint rotations describe the unrefined hands.

Where

sam_3d_body/models/meta_arch/sam3d_body.py (line numbers as of main today):

# Re-run forward
with torch.no_grad():
    verts, j3d, jcoords, mhr_model_params, joint_global_rots = (   # L1599
        self.head_pose.mhr_forward(..., return_joint_rotations=True)
    )
    ...
    pose_output["mhr"]["pred_keypoints_3d"] = j3d          # L1618
    pose_output["mhr"]["pred_vertices"]     = verts
    pose_output["mhr"]["pred_joint_coords"] = jcoords
    pose_output["mhr"]["pred_pose_raw"][...] = 0
    pose_output["mhr"]["mhr_model_params"]  = mhr_model_params   # L1624
    # joint_global_rots is unpacked and dropped

joint_global_rots was last written at L1287 from the body decoder's pass, and nothing updates it after fusion.

Impact

Anything consuming rotations rather than vertices gets hands pinned near the pose prior. In our case (driving a skeletal rig from the predicted rotations) fists render half-open and open hands half-closed for the whole clip, while the refinement sat unused in the same output.

How it was found

Per-joint Kabsch fits of the model's own pred_vertices against its own joint_global_rots, over a 121-frame clip:

  • body joints: consistent to
  • finger joints: inconsistent by 55–123°

i.e. the finger surface is moving in a way the reported finger rotations cannot explain. Confirmed in source afterwards.

Fix

Store the value that is already computed:

     pose_output["mhr"]["mhr_model_params"] = mhr_model_params
+    pose_output["mhr"]["joint_global_rots"] = joint_global_rots

(If the camera-system sign convention applied to verts/j3d/jcoords on L1615-1617 also applies to rotations, that would need matching treatment — worth a maintainer's eye.)

We currently work around it downstream by re-running one batched mhr_forward over the stored post-fusion params and replacing the rotations ourselves. Verified on a 121-frame clip: body joints identical to 0.0°, fingers gain 50–95°, and the result matches the video frame-by-frame (cross-checked against an independent hand estimator).

Happy to send a PR if the one-line store is the fix you want.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions