feat(t2m): locomotion facing/orientation cull gates (#838)#919
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds quaternion-based hip orientation metrics and applies yaw, pitch, and inversion thresholds to forward-facing locomotion clips in ChangesHip Directionality Filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fbee53385
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for f in quats: | ||
| fx, fy, fz = _quat_fwd(f[HIP]) | ||
| ys.append(math.degrees(math.atan2(fx, fz))) | ||
| return abs(sum(ys) / len(ys)) if ys else 0.0 |
There was a problem hiding this comment.
For backward or off-axis locomotion whose hip yaw jitters across the ±180° boundary, this arithmetic mean cancels the angles before the abs() is applied (e.g. +179° and -179° average to 0°), so a clip that should exceed the 25° directionality threshold is kept and can still render as backwards/sideways walking. Compute the circular mean or average angular distance from +Z instead of averaging signed degrees directly.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build-motion-library-v5.py`:
- Around line 199-207: Update _mean_hip_yaw_deg to compute the hip yaw using a
circular mean of the per-frame atan2 angles, avoiding ±180° branch-cut
cancellation. Remove the unused fy binding from the _quat_fwd result while
preserving the existing degree conversion and empty-input behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20179a52-6042-4e49-8933-4fcf546416c8
📒 Files selected for processing (1)
scripts/build-motion-library-v5.py
Walk/run/sit generated "sideways" or face-planted because matchAmong could roll a stylized/quadruped source clip (Fox Warriors, Gynoid, SpongeBob, Spider-Man dive runs) whose locomotion is baked off-axis. The library is rotation-only (root locked at retarget), so a hip baked facing sideways renders as walking sideways; the existing quality gate measured uprightness/ energy but NOT facing direction, so these scored 0.80-0.92 and passed. Add three hip-vector gates, scoped to FORWARD_FACING actions (walk/run/ march) where upright-and-forward is unambiguous — gestures/dance/crouch legitimately lean, and the spine-based topple gate already guards those: - yaw > 25° → faces sideways (clean human ≤10°, stylized rigs 38-153°) - pitch > 30° → diving/lunging (clean ≤17°, superhero dive runs 68-79°) - inverted-frame fraction > 15% → mis-oriented rig renders head-down All reads are on the hip world forward/up vectors: mirror-invariant and rig-agnostic. A signed knee-flex "hyperextension" gate was prototyped and REJECTED — the quat sign isn't mirror-invariant (false-positives on clean left legs) and it caught zero locomotion clips the yaw gate didn't. Library 115 → 109 clips (walk 10, run 8 — all upright/forward now); gestures unaffected. Render-verified: 8/8 walk+run rolls clean, front- facing, proper stride (was ~1/3 sideways before). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7fbee53 to
97ce5e0
Compare
Review (chatgpt-codex P2 + coderabbit): _mean_hip_yaw_deg used an arithmetic mean of signed angles, which cancels across the ±180° wrap — a backward/ off-axis walk jittering +179°/−179° averages to ~0° and wrongly PASSES the 25° directionality cull, still rendering backwards/sideways. Switch to a circular mean (atan2 of summed sin/cos): [179,−179] now reads 180° (culled) while genuine forward [1,−1] stays 0° (kept). Pitch is unaffected (asin, bounded ±90°, no wrap). Library output unchanged on the current corpus (no wraparound cases were being missed) — this hardens the gate for future clips. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed in 2a20a88 — thanks, good catch. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build-motion-library-v5.py`:
- Around line 199-215: Update _mean_hip_yaw_deg to compute
abs(math.degrees(math.atan2(fx, fz))) for each frame and return the arithmetic
mean of those per-frame absolute yaw errors, removing the circular sin/cos
aggregation. In scripts/build-motion-library-v5.py lines 234-243, apply the same
per-frame absolute-value averaging to pitch so opposing signed pitch frames do
not cancel; preserve the existing empty-input behavior and thresholds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0bbb093-9a3f-433f-8306-0ba6ac362fa7
📒 Files selected for processing (1)
scripts/build-motion-library-v5.py
Follow-up review (coderabbit, Major): even the circular-mean yaw still hides OPPOSING bad frames — a clip jittering +90°/−90° yaw has a zero resultant, and pitch +70°/−70° averages to 0° (and stays under the inversion cutoff), so an off-axis clip passes. Fix both gates to take the |deviation| PER FRAME and then average, instead of |aggregate of signed values|: yaw = mean over frames of |atan2(fwd.x, fwd.z)| pitch = mean over frames of |asin(fwd.y)| Verified: +90/−90 yaw → 90° (culled), +70/−70 pitch → 70° (culled), genuine forward +2/−2 → 2° (kept). Current-corpus library unchanged (109 clips) — a robustness hardening. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in the follow-up commit — you're right that even the circular mean hides opposing frames. Both gates now take the per-frame |deviation| then average, not |aggregate|: |
|
1 participant
Summary
Fixes the walk/run/sit "moves sideways / face-plants" problem reported after the 3.23.0 corpus expansion.
Root cause (measured): the motion library is rotation-only (root locked at retarget), so a source clip whose hip is baked facing sideways renders as walking sideways.
matchAmongpicks randomly among an action's clips, so ~1/3 of walk/run rolls landed a stylized/quadruped rig (Fox Warriors, Gynoid, SpongeBob, Spider-Man dive runs) with off-axis locomotion. The existing quality gate measured uprightness/energy but not facing direction — so these scored 0.80–0.92 and passed.Fix
Three hip-vector gates in
clip_quality, scoped toFORWARD_FACINGactions (walk/run/march) where "upright and forward" is unambiguous (gestures/dance/crouch legitimately lean; the spine-based topple gate already guards those):All reads are on the hip world forward/up vectors — mirror-invariant and rig-agnostic.
A signed knee-hyperextension gate was prototyped (the user also reported backward-bending knees) and rejected: the quaternion flex sign isn't mirror-invariant (false-positives on clean left legs — dropped good Quaternius idle/jump), and it caught zero locomotion clips the yaw gate didn't already catch. A reliable knee check needs joint positions and belongs in a future retargeting-math (foot-lock/IK) pass.
Result
QtMeshEditor-models/motion/, 109 clips live).Context
Backed by a research pass on how text-to-motion evolved under a permissive-license constraint (AMASS/HumanML3D are non-commercial → no usable pretrained weights; data + retargeting math dominate). This gate is the highest-leverage deterministic win; retargeting-math improvements (root-align, foot-lock) and more clean CC0/CC-BY data (100STYLE is CC-BY + locomotion-dense) are the tracked next steps.
🤖 Generated with Claude Code
Summary by CodeRabbit