Repository navigation
Conversation
e93fc54 to
d470b0b
Compare
1afe113 to
ce16716
Compare
d470b0b to
75f90fe
Compare
402c544 to
e52d66c
Compare
75f90fe to
07d7a99
Compare
c3ae357 to
c5f561f
Compare
9339bac to
cfeee2a
Compare
cfeee2a to
e3becc1
Compare
c5f561f to
c09f941
Compare
c09f941 to
e9f4073
Compare
1fb4886 to
c27cd6d
Compare
0f56bf9 to
028cd76
Compare
c27cd6d to
0a796ff
Compare
RyanPCo
left a comment
There was a problem hiding this comment.
Automated adversarial review (read-only; findings were checked by a second skeptical pass before posting): 1 low, 2 nit.
| 5. Render side-by-side: left half = Aria viz with Aria.FINGER_EDGES; right | ||
| half = fitted MANO viz with Human (=Mecka) FINGER_EDGES. | ||
| 5. Render side-by-side: raw Aria keypoints on the left, the fitted MANO on | ||
| the right, both through Human.viz(mode="keypoints"). |
There was a problem hiding this comment.
low (correctness): The left panel still feeds Aria's raw 21-slot layout (tips at 0-4, palm root at 5; that is what the deleted ARIA_FINGER_EDGES described) into Human.viz(mode="keypoints"). _viz_keypoints_horizon_trace hard-codes FINGERTIP_SLOTS=(4,8,12,16,20). The wrist comes out right because 138-D reads it from the wrist block, but the 'thumb' trace is now the Aria pinky tip and the other four are mid-finger joints, so the comparison panel is mislabelled. Either pass Aria's tip slots through the existing fingertip_indices kwarg (e.g. fingertip_indices=(0,1,2,3,4)), permute the batch to MANO order first, or drop the left panel. The docstring line should then say which of these it does.
| f"got {actions.shape}" | ||
| ) | ||
|
|
||
| if actions.shape[-1] == 2 * (3 * _N_KP + 9): |
There was a problem hiding this comment.
nit (simplification): Widths are now listed twice: _KEYPOINT_WIDTHS (built from the pose sizes) and this elif chain, which writes 144/140/138 as 2 * (3 * _N_KP + n) and is harder to read than the literals it replaced. A single table such as _SPLIT_KWARGS = {144: dict(wrist_in_data=True, is_rot6d=True), 140: dict(wrist_in_data=True), 138: dict(wrist_in_data=True, is_quat=False), 126: dict(wrist_in_data=False)} could drive both the validation and the _split_keypoints(actions, **_SPLIT_KWARGS[D]) call.
| colors = cls.FINGER_COLORS | ||
| dot_color = cls.DOT_COLOR | ||
| return _viz_keypoints( | ||
| # The horizon trace is the ONLY keypoint renderer. A chunk draws |
There was a problem hiding this comment.
nit (comments): This comment narrates history ('rendered motionless keypoint overlays for a whole 50k-step campaign'). The dated anecdote in the tests/unit/test_chunk_renderer.py docstring does the same. Cut it to the constraint, e.g. 'Single keypoint renderer; a (D,) step draws as start markers', and put the story in the commit message. The np.asarray(viz_data) here is redundant because the callee already does it.
_viz_keypoints flattened a (T, D) chunk to (T * 21, 3) and then drew slots 0..20 only -- `for k in range(21)` for the dots, and edge_ranges over the same 21 -- so an action chunk collapsed to its first timestep and the overlay never moved, however long the horizon. Every projected point past frame 0 was computed and thrown away. That is what a whole training campaign's validation videos turned out to be (2026-09-18). This replaces it with _viz_keypoints_horizon_trace, which draws the chunk as a trajectory and degenerates to per-slot start markers for a lone timestep (it promotes (D,) to (1, D) itself). The old renderer is deleted rather than left reachable: Human.viz(mode="keypoints") has one path, and a renderer you can reach by accident is one you will reach. Consequences, all intended: the dataset inspector, the Aria tutorial and mecka_keypoints_viz render through the trace now, so the single-frame callers show per-slot start markers instead of a posed skeleton, and Aria's raw 21-keypoint edge tables go with the renderer that consumed them -- they had no other caller. tests/unit/test_chunk_renderer.py pins both halves: that the trace follows the chunk, and that no second renderer is reachable from Human. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01THTuWdgk6M6pZ5Ya7ihZEF
028cd76 to
1c42a93
Compare
0a796ff to
17231a2
Compare
|
Review findings: status after the fixup pass. The fixes are squashed into this PR's commits, and the whole stack was force-pushed. Fixed
|

_viz_keypoints flattened a (T, D) chunk to (T * 21, 3) and then drew slots
0..20 only --
for k in range(21)for the dots, and edge_ranges over the same21 -- so an action chunk collapsed to its first timestep and the overlay never
moved, however long the horizon. Every projected point past frame 0 was
computed and thrown away. That is what a whole training campaign's validation
videos turned out to be (2026-09-18).
This replaces it with _viz_keypoints_horizon_trace, which draws the chunk as a
trajectory and degenerates to per-slot start markers for a lone timestep (it
promotes (D,) to (1, D) itself). The old renderer is deleted rather than left
reachable: Human.viz(mode="keypoints") has one path, and a renderer you can
reach by accident is one you will reach.
Consequences, all intended: the dataset inspector, the Aria tutorial and
mecka_keypoints_viz render through the trace now, so the single-frame callers
show per-slot start markers instead of a posed skeleton, and Aria's raw
21-keypoint edge tables go with the renderer that consumed them -- they had no
other caller.
tests/unit/test_chunk_renderer.py pins both halves: that the trace follows the
chunk, and that no second renderer is reachable from Human.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw