examples/mujoco_xr: the ghost renders the governed pose, and says so - #928
Open
jiwenc-nv wants to merge 4 commits into
Open
examples/mujoco_xr: the ghost renders the governed pose, and says so#928jiwenc-nv wants to merge 4 commits into
jiwenc-nv wants to merge 4 commits into
Conversation
…rvo followers EePoseRateLimiter, JointRateLimiter and RateLimiterConfig bound the per-frame step of a command stream before it reaches a follower with bare position servos -- an SO-101 executes whatever reaches the bus at full torque, so an IK divergence or a tracking teleport becomes a full-speed slew. Three bands: motion under the limits passes through untouched, motion over them is clamped, and a frame whose input velocity breaks the reject envelope is refused outright rather than approached. Cherry-picked from #727 (5 commits, squashed). The tree is byte-identical to that branch; only the location differs, and git's own rename detection placed it: #727 targets src/retargeters/, and the package now lives at src/python/isaacteleop/retargeters/. The __init__.py registration is the automerge of #727's against the current file, so WujiHandRetargeter -- which postdates #727 -- survives. 60 tests in this file pass; the CMakeLists globs test_*.py, so they register without a build change. Squashed rather than replayed as five commits because three of them are GitHub web edits carrying no Signed-off-by, which the DCO check rejects and which only the author can supply. The author field carries his NVIDIA identity rather than the personal address on #727's commits. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
The ghost's pose now comes from an EePoseRateLimiter rather than straight off the controller, so what the operator sees is the command a follower would execute. That alone is not perceptible -- under the limits the limiter is a pass-through, and over them a clamp and a refusal both read as "the tool is behind my hand". So InterventionMonitor recovers the band by comparing what the limiter was handed against what it emitted, and recolours the ghost: amber while clamping, red while rejecting. Colour goes on the shared material, not per geom, and alpha stays 1.0 in every band -- leader_gripper.xml's opacity is what keeps draw order free and the ghost out of CloudXR's reprojection buffer. The limits are chosen for this demo, not measured against an SO-101: ordinary reaching passes through, a deliberate flick trips both bands. test_harness.py drives the real chain at those numbers, so retuning them past what a hand can reach fails rather than silently producing a demo that never intervenes. Tracking loss keeps its old contract, and it needs a gate of its own now: the limiter holds a pose for the body indefinitely, so without one the jaw would go on following the trigger and articulate the ghost on a stale pose. _loop freezes the whole gripper when the limiter's INPUT is absent, not its output. SO101ClutchRetargeter is the shipped producer of this ee_pose contract and is deliberately unused -- it re-bases onto a follower's base frame at every engage, and a leader in the operator's hand has no home to clutch to. GripPoseSource adapts the controller instead, and goes absent on an untracked grip rather than emitting the scene origin. A second translucent ghost showing the raw pose was rejected: the renderer draws with CULL_MODE_NONE, LEQUAL and unconditional depth writes, so a coincident copy wins wherever the two agree -- which is the pass-through case, i.e. exactly when nothing is wrong. Prerequisite for #738. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
9 tasks
…valid grip Every session crashed on its first frame with "Tensor 'pose' value has not been set". The grip pose is not localizable for the first frames of a session, so GripPoseSource goes absent, EePoseRateLimiter has nothing to latch, and it returns without writing its output. That output is a REQUIRED group, and TensorGroup.is_none is hardcoded False for those, so the group reads as present from frame zero while the tensor inside it is still unset -- invisible to the is_none check that correctly catches the same absence on RAW_POSE_KEY. Tensor exposes no "has it been set" predicate, so the raise is the only signal there is to read. The tests missed it because every one of them handed the chain a valid grip on the first frame, which is the one thing a real session never does. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
Found on a headset: the ghost flew off after about a second, with float32 overflow in astype() and then NaN through every quaternion product. _clamp_orientation_step's pass-through branch returns prev (x) (prev^-1 (x) target) -- target, re-derived through prev to keep the hemisphere. Its norm is |prev|^2, and it becomes the next frame's prev, so the deviation from unit SQUARES every frame. From float64's 1e-16 that reaches float32's ceiling in about 60 frames, i.e. 0.9 s at 72 Hz, and the pose goes non-finite. Only pass-through diverges; the clamp branch multiplies by a unit step and holds. So the band a well-tuned harness spends its entire session in was the one that blew up, which is why the existing tests -- all a handful of frames, mostly clamping -- did not see it. Bug is in the code cherry-picked from #727 and reaches any consumer of EePoseRateLimiter, not just mujoco_xr. Measured after the fix: 20000 frames of jittered pass-through hold ||q|-1| at 2.2e-16. Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stacked on #900 — review that first; this diff is only the two commits on top.
The ghost gripper's pose now comes from an
EePoseRateLimiterrather than straight off the controller, so what the operator sees in the headset is the command a follower would execute, not where their hand is. That alone isn't perceptible: under the limits the limiter is a pass-through, and over them a clamp and a refusal both read as "the tool is behind my hand". SoInterventionMonitorrecovers the band by comparing what the limiter was handed against what it emitted, and recolours the ghost — amber clamping, red rejecting. Colour goes on the shared material (one write recolours the tool;geom_rgbawould silently win over it) and alpha stays 1.0 in every band, becauseleader_gripper.xml's opacity is what keeps draw order free and the ghost out of CloudXR's reprojection buffer.Prerequisite for #738, whose thesis is that operators lose sessions to interventions they cannot perceive.
Three calls to confirm rather than inherit. (1) A second translucent ghost showing the raw pose would be the obvious contrast, and is wrong here: the renderer draws with
CULL_MODE_NONE,LEQUALand unconditional depth writes, so a coincident copy wins wherever the two agree — the pass-through case, i.e. exactly when nothing is wrong. (2)SO101ClutchRetargeteris the shipped producer of thisee_posecontract and is unused, because it re-bases onto a follower's base frame at every engage and a leader in the operator's hand has no home to clutch to;GripPoseSourceadapts the controller instead. (3) The limits are chosen for this demo, not measured against an SO-101 — 0.5 m/s and 2.5 rad/s clamp, 2.0 m/s and 10 rad/s reject, set so ordinary reaching passes through and a deliberate flick trips both bands.RateLimiterConfigitself defaults to 0.25 m/s, which would clamp during normal reaching and make the demo read as lag.The first commit is #727 cherry-picked (5 commits, squashed), byte-identical to that branch — git's own rename detection did the rehoming from
src/retargeters/tosrc/python/isaacteleop/retargeters/, and the__init__.pyregistration is the automerge against the current file, soWujiHandRetargetersurvives. Squashed because three of #727's commits are web edits with noSigned-off-by. If #727 lands first this will conflict, and that ordering is yours to decide.Type of change
Testing
Ubuntu 22.04 / aarch64 (Tegra).
ctest -L mujoco_xr5/5 (newtest_harness.py, 17 tests) andctest -R retargeting_26/26 including mypy and the 60 rehomed limiter tests.test_harness.pydrives the realGripPoseSource → EePoseRateLimiterchain at the app's own_HARNESSlimits, so retuning them past what a hand can reach turns three tests red rather than silently producing a demo that never intervenes. It also asserts the band reaches themjvGeomrgba the Vulkan draw loop actually memcpys rather thanmat_rgba, sincegeom_rgbawould override the material and an assertion on the model would prove nothing. It executes_build_pipeline()too — the graph fans the controller out to two consumers and exposes one port under two combiner names, neither obviously legal, and a connect-time rejection would otherwise surface only on a headset.One regression this caught and fixed: under tracking loss the jaw kept following the trigger while the body froze, articulating the ghost on a stale pose.
_loopnow freezes the whole gripper when the limiter's input is absent, not its output.Not verified, and not verifiable without hardware: whether an amber gripper reads as "the harness is holding you back" to someone wearing the headset. No
mujoco_xrtest can run in CI yet — nothing in.github/workflows/installsmujoco(#880).Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCO