Feat/microduck sim - #3937
Open
Nabla7 wants to merge 5 commits into
Open
Conversation
Nabla7
marked this pull request as ready for review
September 4, 2026 12:42
Nabla7
requested review from
Dreamsorcerer,
aclauer,
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
September 4, 2026 12:42
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
Greptile SummaryThis PR adds a complete MicroDuck MuJoCo simulation stack, including seven ONNX policies, whole-body control and shared-memory adaptation, robot configuration, blueprint registration, Rerun MJCF visualization, documentation, and integration tests.
Confidence Score: 4/5The PR should not merge until E-stop and disarm replace or clear previously latched actuator targets. The new task stops producing output when disabled, but MuJoCo continues applying its last position command, allowing joints to continue moving toward stale targets after E-stop. Files Needing Attention: dimos/control/tasks/microduck_policy_task/microduck_policy_task.py Important Files Changed
Reviews (1): Last reviewed commit: "Merge branch 'main' into feat/microduck-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+352
to
+353
| if not self._active or not self._armed or self._estopped: | ||
| return None |
Contributor
There was a problem hiding this comment.
When E-stop, disarm, or an inference failure makes compute() return None, arbitration sends no replacement command for these joints. MuJoCo then continues applying its previously latched position target on every physics step. If a joint has not reached that target, it can continue moving after E-stop instead of becoming inert within one coordinator tick. Please emit an explicit safe hold/stop command or clear the adapter's latched command when disabling output.
Knowledge Base Used:
aromeoes
added a commit
that referenced
this pull request
Sep 4, 2026
Every dimos package named after a specific robot model sits two levels deep,
under its vendor or class: unitree/{b1,g1,go2}, manipulators/{a1z,a750,
dual_openyam,openarm,openyam,piper,xarm}, diy/alfred - eleven of eleven. Ours
was the only one placed flat, and the flat path also discarded provenance our
own code already knows: assets_fetch.py pulls from pollen-robotics/microduck
and pollen-robotics/microduck_rl.
PR #3937, which adds Microduck support independently, puts it at
dimos/robot/pollen/microduck/. That is the unitree/g1 shape and it is right, so
follow it. A single-tenant vendor directory has precedent in diy/alfred, so the
"extra empty level" objection does not apply. No __init__.py: the repo uses
implicit namespace packages throughout dimos/robot.
Mechanical: git mv plus 49 import references and 5 path strings; the blueprint
registry regenerated to match. Blueprint and module resolution verified for all
six registry entries.
Also fixes a fragility a review flagged in the render_depth change: depth_needed
was bound AFTER the add_camera closure that reads it. It worked only because
the sole call site sits below the assignment, so any future caller placed above
it would have raised NameError. Bound before the closure now, and primary_needed
reduced to `enable_color or depth_needed` - it had been spelling out the same
two conditions a second time.
Tests: 970 python + 24 mujoco-marked, ruff clean.
Contributor
|
lol good idea |
TomCC7
requested changes
Sep 4, 2026
Member
There was a problem hiding this comment.
we now support pulling robot asset from remote directly :), so better use that instead of copying files to our repo: https://docs.dimensional.org/capabilities/manipulation/adding_a_custom_arm/#4a-add-your-urdf
| from numpy.typing import NDArray | ||
|
|
||
| _Vec3 = NDArray[np.float64] | ||
| _Quat = NDArray[np.float64] |
Member
There was a problem hiding this comment.
are we doing all the quaternion calculation by hand?
Member
There was a problem hiding this comment.
we have these in repo better align with those:
from dimos.msgs.geometry_msgs.Quaternion import Quaternion
dimos.utils.transform_utils|
|
||
| # rest position, rest quaternion (wxyz), then a rotation about local +z | ||
| _HEAD_CHAIN: tuple[tuple[tuple[float, ...], tuple[float, ...]], ...] = ( | ||
| ((0.026, 0.0145, 0.0324215), (0.0, 0.0, 0.707107, -0.707107)), |
Member
There was a problem hiding this comment.
sorry for being pedantic I just feel like these math representation deserves more unified types... this is just an SE3 pose with quaternion as rotation repr so should be quite standard type
| ((0.0, 0.0186931, -0.0145), (0.0, 0.0, -0.707107, -0.707107)), | ||
| ((-0.0179, 0.0, 0.0145), (0.707107, 0.0, -0.707107, 0.0)), | ||
| ) | ||
| _CAMERA_POS = np.asarray((0.0155, -9.13778e-05, -0.0733), dtype=np.float64) |
Member
There was a problem hiding this comment.
are these information not present in the mjcf? if so shouldn't we use proper fk lib for these calculation like pinocchio?
| @dataclass | ||
| class MicroDuckPolicyTaskConfig: | ||
| policy_dir: str | Path | ||
| joint_names: list[str] |
Member
There was a problem hiding this comment.
we should only put variables that's really changable in the config. if joint_names are never gonna change why not just hardcode as a constant? also can avoid the action_len check below
Comment on lines
+139
to
+144
| if config.timeout <= 0.0: | ||
| raise ValueError("MicroDuck velocity timeout must be positive") | ||
| for field_name in ("command_alpha", "head_alpha", "body_alpha"): | ||
| value = float(getattr(config, field_name)) | ||
| if not 0.0 < value <= 1.0: | ||
| raise ValueError(f"{field_name} must be in (0, 1]") |
Member
There was a problem hiding this comment.
these config verification can be done in pydantic field declaration or custom validator if you declare config to be a pydantic model or dataclass
Comment on lines
+201
to
+206
| expected = { | ||
| "schema_version": 2, | ||
| "model_api": 1, | ||
| "obs_len": OBS_LEN, | ||
| "action_len": ACTION_LEN, | ||
| } |
Member
There was a problem hiding this comment.
should just declare a pydantic model for this and use it's serialization directly
| ) | ||
|
|
||
| @staticmethod | ||
| def _xyzw(quaternion_wxyz: Any) -> list[float]: |
Member
There was a problem hiding this comment.
bunch of Any across the PR, most of them can be actually properly typed
Comment on lines
+163
to
+172
| { | ||
| ("cmd_vel", Twist): LCMTransport("/microduck/cmd_vel", Twist), | ||
| ("microduck_joints", JointState): LCMTransport("/microduck/joints", JointState), | ||
| ("imu", Imu): LCMTransport("/microduck/imu", Imu), | ||
| ("odom", PoseStamped): LCMTransport("/microduck/odom", PoseStamped), | ||
| ("color_image", Image): pSHMTransport("/microduck/color_image"), | ||
| ("depth_image", Image): pSHMTransport("/microduck/depth_image"), | ||
| ("camera_info", CameraInfo): LCMTransport("/microduck/camera_info", CameraInfo), | ||
| ("depth_camera_info", CameraInfo): LCMTransport( | ||
| "/microduck/depth_camera_info", CameraInfo |
Member
There was a problem hiding this comment.
are you testing on macos? that explains the manual lcm override lol. we need to fix zenoh
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
3 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.
What
Adds
microduck-sim: the official MicroDuck MJCF and seven non-roller ONNX policies running in MuJoCo, with Rerun visualization and controls.Run
Test
WASD locomotion, sit/stand, head control, one-shot motions, E-stop, and reset.
AI assistance
Codex (GPT-5), extensive.