-
Notifications
You must be signed in to change notification settings - Fork 12
Add the MolmoSpaces env-server integration #504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import logging | ||
| from pathlib import Path | ||
|
|
||
| import configuronic as cfn | ||
|
|
||
| from positronic.cfg.eval import number_trials, spec | ||
| from positronic.drivers.roboarm.models import GRASP_SITE_LINK, bundled_franka_model | ||
| from positronic.eval import Eval, Observation, Task | ||
| from positronic.eval import keys as eval_keys | ||
| from positronic.simulator.env_server.proxy import RemoteEnvControlSystem, remote_franka_embodiment | ||
| from positronic.simulator.molmo_spaces import keys as molmo_keys | ||
| from positronic.simulator.molmo_spaces import mapping | ||
| from positronic.simulator.molmo_spaces.adapter import DEFAULT_CAMERA_DICT, MolmoAdapter | ||
| from positronic.simulator.molmo_spaces.launcher import serve_molmo_spaces | ||
|
|
||
| # How far the harness deadline sits above the benchmark horizon. Being sim-time, the spare budget costs | ||
| # nothing unless the sim stops terminating, which is the only thing the deadline is there to catch. | ||
| _TIMEOUT_MARGIN_SEC = 10.0 | ||
|
|
||
|
|
||
| @cfn.config(camera_dict=DEFAULT_CAMERA_DICT, episodes=None, trial_count=1, timeout=None, seed=None) | ||
| def _molmo_eval( | ||
|
vertix marked this conversation as resolved.
|
||
| benchmark_dir: str, | ||
| episodes: int | list[int] | None, | ||
| trial_count: int, | ||
| timeout: float | None, | ||
| camera_dict: dict[str, str], | ||
| seed: int | None, | ||
| ) -> Eval: | ||
| """A MolmoSpaces eval: the embodiment proxies a remote MolmoSpaces env, the task carries the scenario. | ||
|
|
||
| MolmoSpaces (https://github.com/allenai/molmospaces) is AllenAI's MuJoCo manipulation benchmark on the DROID | ||
| rig (Franka arm + Robotiq 2F-85) across ProcTHOR scenes; a benchmark is a directory holding a ``benchmark.json`` | ||
| (a JSON list of episode specs — house, task, exact object poses, cameras, language goal), so | ||
| ``--eval.benchmark_dir`` names that directory and ``--eval.episodes`` optionally pins a subset of episode | ||
| indices (default: the whole benchmark). The asset packs live under ``MLSPACES_ASSETS_DIR``. | ||
|
|
||
| positronic launches a single task-agnostic env server in MolmoSpaces' own interpreter; the proxy drives it | ||
| over the socket, the env answers which episodes the sweep runs, and the episode index rides each trial's reset | ||
| token. The instruction is never pinned: the task reads its language live from the env, which reports the | ||
| episode's resolved goal in every reset's meta. Episodes are exact-pose deterministic, so ``trial_count`` | ||
| defaults to 1. | ||
|
|
||
| ``timeout`` is not the benchmark horizon — the sim owns that (the benchmark's ``task_horizon_sec``, enforced | ||
| env-side and delivered as a terminal ``done``). It is only a runaway-cost safety net for a sim that never | ||
| terminates, so its default is the benchmark's own horizon plus a margin. An explicit value can only lower the | ||
| deadline, never raise it, and one at or below the horizon truncates valid episodes — so any value that | ||
| differs from the default is warned about. | ||
| """ | ||
| # A non-positive count yields no trials at all, and an empty plan reads to the self-driving harness as a | ||
| # finished run — the command would exit 0 having evaluated nothing. | ||
| if trial_count < 1: | ||
| raise ValueError(f'--eval.trial_count must be at least 1, got {trial_count}') | ||
| proxy = RemoteEnvControlSystem(MolmoAdapter(camera_dict), serve_molmo_spaces(Path(benchmark_dir))) | ||
| # MolmoSpaces drives a Franka DROID rig; recordings carry the same model (URDF + meshes + joint names + | ||
| # control frame) for the 3D viewer and offline IK, supplied here since the molmo server can't import | ||
| # positronic to emit it via ``robot_meta``. ``DEFAULT_FRAME`` is declared on the gripper's grasp site, | ||
| # which is where ``env.py`` reports ``robot_state.ee_pose`` and resolves Cartesian targets, so a policy | ||
| # frame reached from it via ``ChangeEEFrame`` and offline IK over a recording both anchor correctly. | ||
| embodiment = remote_franka_embodiment( | ||
| proxy, camera_dict, descriptor='remote.molmo_spaces.droid', static_meta=bundled_franka_model(GRASP_SITE_LINK) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Rule hidden-dependency violated: AGENTS.md reference: AGENTS.md:L7-L8 Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferring — the coupling is real, the fix is a protocol change that should not ride this PR. You have the mechanism right: What holds today is narrower than the invariant: MolmoSpaces ships one rig (the Franka DROID arm + Robotiq 2F-85) across every benchmark, so the site name determines the transform in practice. That makes this latent rather than live. Closing it properly means a new reset-frame field carrying the live flange-to-grasp transform, the env computing it, and
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Owner's steer: keep a copy of MolmoSpaces' own robot description in the positronic wrapper and build the recorded model from it, so both sides derive from one source instead of positronic supplying a look-alike FR3 and the two agreeing by name. Simpler than the wire transform, and it removes the mismatch by construction. Tracked as #593, with the open questions that need the asset packs to answer — whether they distribute a URDF or only MJCF, whether the grasp site survives as a frame in it, and licensing/size of the vendored assets. The wire-transform version is recorded there as the fallback. |
||
| ) | ||
| # The env's full MuJoCo state is recorded as privileged ground truth, never fed to the policy. | ||
| privileged = {mapping.OBS_SIM_STATE: Observation(proxy.privileged[mapping.OBS_SIM_STATE], None)} | ||
|
|
||
| def tasks() -> list[Task]: | ||
| params = proxy.tasks(spec(episodes=episodes)) | ||
| # The benchmark declares one horizon over all its episodes (the env refuses an inconsistent one), so one | ||
| # backstop deadline covers the run. | ||
| backstop = params[0][molmo_keys.TASK_HORIZON] + _TIMEOUT_MARGIN_SEC | ||
| if timeout is not None and timeout != backstop: | ||
| logging.warning( | ||
| '--eval.timeout %ss overrides the benchmark backstop of %ss (the %ss horizon plus a margin); ' | ||
| 'running with %ss. The deadline only catches a sim that stopped terminating, and a deadline at ' | ||
| 'or below the horizon cuts valid episodes short and scores them as failures.', | ||
| timeout, | ||
| backstop, | ||
| params[0][molmo_keys.TASK_HORIZON], | ||
| min(timeout, backstop), | ||
| ) | ||
| deadline = backstop if timeout is None else min(timeout, backstop) | ||
| task = Task(instruction_source=lambda: proxy.meta[mapping.META_TASK], timeout_sec=deadline) | ||
| # Benchmark episodes are exact-pose deterministic and carry their own seed. An unset ``seed`` leaves | ||
| # ``eval.seed`` off the trial, so the env falls back to the episode's spec seed (reproducing the | ||
| # benchmark); an explicit ``seed`` overrides it, sweeping ``seed .. seed + trial_count - 1``. | ||
| return number_trials([ | ||
| (task, {**p, **({eval_keys.SEED: seed + t} if seed is not None else {})}) | ||
| for p in params | ||
| for t in range(trial_count) | ||
| ]) | ||
|
|
||
| return Eval(embodiment, tasks, privileged=privileged, done=proxy.done) | ||
|
|
||
|
|
||
| # The whole benchmark in one run (every episode in ``--eval.benchmark_dir``'s benchmark.json). | ||
| benchmark = _molmo_eval | ||
|
|
||
| # A single-episode smoke target: the first episode of the benchmark. | ||
| first_episode = _molmo_eval.override(episodes=0) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,35 @@ | |
|
|
||
| This package implements the protocol and utilities for offboard policy inference, allowing robots or simulators to stream observations to a remote server and receive actions. | ||
|
|
||
| ## Separation of responsibilities: adapter vs codec vs wire client | ||
|
|
||
| Three layers touch an observation on its way to a model, and each owns exactly one concern. | ||
| When writing a new sim/rig adapter, check this table before adding any transform to it: | ||
|
|
||
| | Layer | Owns | Examples | | ||
| |---|---|---| | ||
| | **Adapter** (per sim/rig, e.g. `simulator/molmo_spaces/adapter.py`) | Rig semantics ONLY: mapping the rig's observation/action vocabulary onto positronic's raw keys | Camera-key mapping, gripper qpos → `[0, 1]` closure, decoded commands → the rig's action format | | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adapter is the detail of sims, and has nothing in common with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed on the substance, and the fix is bigger than a wording change — leaving this open for your call on where it lands. Two separate things are wrong with that row, and you named both:
The section's audience is whoever writes an adapter, so the honest home looks like I have not done it: the section is load-bearing for adapter authors and splitting it across two READMEs is a judgement about which half each audience needs, which is yours. Say the word on the destination and I will move it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Rule stale-doc violated: AGENTS.md reference: AGENTS.md:L7-L22 Useful? React with 👍 / 👎. |
||
| | **Codec** (per model family, `policy/codec.py` subclasses) | Model preprocessing: everything the checkpoint's input distribution requires | Resize-with-pad to model resolution, prompt normalization (e.g. DROID lowercasing), state assembly | | ||
| | **Wire client** (`InferenceClient` / `RemotePolicy`) | Transport optimization, negotiated — never semantics | Downscaling frames to the server-advertised `image_sizes` (aspect-preserving, never upscaling), optional JPEG compression | | ||
|
|
||
| Consequences: | ||
|
|
||
| - **An adapter never resizes, pads, normalizes prompts, or otherwise preprocesses for the model.** | ||
| It passes frames and text through at native fidelity. If the same transform appears in an adapter | ||
| and a codec, the adapter's copy is the bug: a drifted duplicate silently changes eval inputs. | ||
| - **Bandwidth is not the adapter's problem.** The client already downsizes to what the server says | ||
| it needs: every `Codec` advertises its expected input sizes via the reserved `image_sizes` meta | ||
| key (see `Codec.meta`), the server returns it in the session handshake, and the client fits | ||
| frames to it before sending. This is default-on — an adapter that resizes "to keep the wire | ||
|
Comment on lines
+21
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the inspected offboard path, Useful? React with 👍 / 👎. |
||
| payload small" is duplicating it. | ||
| - **Codecs run on either side of the wire** — the client being the process driving the robot or sim, | ||
| the server being the process holding the model. positronic-native evals compose the codec around | ||
| `RemotePolicy` on the client (`cfg/policy.py` — the wire then carries model-sized encoded inputs, | ||
| and the client-side resize is disabled since `codec.meta` already reports `image_sizes`). | ||
| Thin-client deployments (a sim adapter in a foreign venv talking to a serverless endpoint) host | ||
| the codec on the server — the wire carries raw positronic keys, downsized by the negotiation | ||
| above. Both placements are supported; pick by where the dependencies can live. | ||
|
|
||
| ## Protocol v1 | ||
|
|
||
| The unified WebSocket protocol is built to enable ANY hardware to connect to ANY model. All Positronic inference servers (LeRobot, GR00T, OpenPI) implement this protocol, allowing a single `.remote` policy client to work across all vendors. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule stale-doc violated:
ARCHITECTURE.mdsays every sim-env adoption ships a native-vs-Positronic parity test, but a repo-wide search finds such a test only undermolmo_spaces/tests/parity.py; the existing LIBERO and RoboLab scripts validate command transforms rather than comparing complete native and Positronic episode runs. Either add parity coverage for those adoptions or phrase this as a requirement for new adoptions instead of a property that already holds.AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.