Skip to content

Add MolmoSpaces adapter under simulator/molmo_spaces - #495

Closed
vertix wants to merge 9 commits into
mainfrom
molmo-spaces-adapter
Closed

Add MolmoSpaces adapter under simulator/molmo_spaces#495
vertix wants to merge 9 commits into
mainfrom
molmo-spaces-adapter

Conversation

@vertix

@vertix vertix commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Migrate the pi05_droid <-> MolmoSpaces eval adapter from the pilot into the repo, mirroring the simulator/libero package layout.

What moved

  • positronic/simulator/molmo_spaces/adapter.py — the pure obs/action mapping (molmo_obs_to_positronic, positronic_action_to_molmo, resize_with_pad), ChunkBuffer, FakePolicy, and the MolmoSpacesPolicy (InferencePolicy subclass).
  • positronic/simulator/molmo_spaces/tests/test_adapter.py — 20 fixture-driven unit tests.
  • positronic/simulator/molmo_spaces/tests/droid_obs.npz — the committed synthetic DROID observation fixture (962 B).
  • positronic/simulator/molmo_spaces/make_fixture.py — regenerates the fixture (mirrors libero's make_fixture.py; verified byte-array identical to the committed copy).
  • __init__.py at the package root and tests/, mirroring libero.

What was adapted (vs a pure file move)

  • Dependency wiring: no new dependency added. molmo_spaces is imported behind an ImportError guard (HAS_MOLMO_SPACES) and positronic's RemotePolicy is imported lazily inside make_policy_client, so the pure mapping logic and its tests load with neither framework installed — the same guarded-import pattern libero's client-side code uses. MolmoSpaces is not pip-installable and is intentionally kept out of pyproject.toml.
  • Test module: dropped the pilot's PEP 723 # /// script header, sys.path insertion, and standalone runner; imports now resolve through the package (positronic.simulator.molmo_spaces.adapter). The two raise-checks use pytest.raises. The fixture path points at the same tests/ dir (mirroring libero's test_e2e.py).
  • Docstrings/comments: rewrote to state current facts only — stripped dated pilot references while keeping the durable pitfall context they guarded (bytes-keyed msgpack payloads in _wire_get; the trailing horizon-marker drop in ChunkBuffer).
  • testpaths: added positronic/simulator/molmo_spaces/tests to [tool.pytest.ini_options]. (Unlike libero — whose only test is a heavy env-server e2e deliberately excluded from the default suite — this adapter test is lightweight and fixture-driven, so it belongs in the default collection.)

How tests run / skip

uv run --locked pytest positronic/simulator/molmo_spaces/tests/test_adapter.py --no-cov → 20 passed. Every test exercises the pure mapping functions, ChunkBuffer, and FakePolicy, so the suite passes with molmo_spaces absent (the common dev/CI box); the real-sim MolmoSpacesPolicy is only instantiable where the package is installed and is not touched by these tests.

This covers only the adapter half of the MolmoSpaces migration and precedes the merge of nebius-competition #22 per internal#76.

🤖 Generated with Claude Code

Migrate the pi05_droid <-> MolmoSpaces adapter from the eval pilot into the repo,
mirroring the `simulator/libero` package layout: `adapter.py`, `make_fixture.py`,
`__init__.py`, and `tests/` with a committed synthetic fixture.

`molmo_spaces` is imported behind an `ImportError` guard (`HAS_MOLMO_SPACES`) and
positronic's `RemotePolicy` lazily, so the pure mapping logic and its tests need
neither framework installed — the fixture-driven `test_adapter.py` runs on a bare
box. No new dependency is added.

Register the new tests dir in `[tool.pytest.ini_options] testpaths`.

Requested-by: Vladimir Yakunin
Ticket: Positronic-Robotics/internal#76 #refs
@vertix

vertix commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: b6dfc689a9

ℹ️ 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".

@vertix

vertix commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: b6dfc689a9

ℹ️ 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".

@vertix

vertix commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6dfc689a9

ℹ️ 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".

Comment thread positronic/simulator/molmo_spaces/adapter.py Outdated
The Zed-wrist and light-randomized benchmark variants emit
`wrist_camera_zed_mini` / `droid_shoulder_light_randomization` instead
of the default keys; MolmoSpaces' own Pi policy prefers the variant when
present, and hard indexing raised KeyError on those observations. Mirror
the upstream precedence for the default config; an explicitly configured
non-default key stays read as-is.

Ticket: Positronic-Robotics/internal#73 #refs
@vertix

vertix commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 23f1dc65d1

ℹ️ 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".

vyakunin added 2 commits July 23, 2026 08:29
Fixture generators exist only to (re)build committed test fixtures, so they
live beside the fixtures and the tests that consume them. Path references in
docstrings updated; the molmo script's output path is now relative to its
new location (verified byte-identical output).

Requested-by: Vladimir Yakunin
Ticket: none (layout convention change agreed by founders in PR #495 review)
Run commands in docstrings point at the new locations; the molmo script now
writes `droid_obs.npz` next to itself instead of into a `tests/` subdir.

Requested-by: Vladimir Yakunin
Ticket: none (layout convention change agreed by founders in PR #495 review)
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23f1dc65d1

ℹ️ 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".

Comment thread positronic/simulator/molmo_spaces/adapter.py Outdated
MolmoSpaces' custom-policy convention is that the policy config carries
`camera_names` (and `eval_main.py --camera_names` overrides it); a default
`AdapterConfig` ignored it, so an eval configured with non-default camera
names KeyErrored before inference. `MolmoSpacesPolicy` now derives its
camera keys from the config when no explicit `adapter_config` is passed —
the wrist role by the 'wrist' substring, the exterior role otherwise, each
falling back to its default key.

Ticket: none (Codex review finding on PR #495)
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1aac788715

ℹ️ 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".

Comment thread positronic/simulator/molmo_spaces/adapter.py
Comment thread positronic/simulator/molmo_spaces/tests/test_adapter.py
The Pi baseline lowercases the task description before inference
(pi_policy.py:144), so capitalized benchmark text must not reach the
checkpoint mixed-case — `molmo_obs_to_positronic` now lowercases it.
The wiring test's stub config gains `force_enable_depth` (the one field
MolmoSpaces' `BasePolicy.__init__` reads), so the suite also passes where
molmo_spaces is installed and the real superclass path runs.

Ticket: none (Codex review findings on PR #495)
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Unknown error
ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 87472fbaf8

ℹ️ 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".

Per the founders' design decision: an adapter carries rig semantics only —
model preprocessing belongs to the codec, wire optimization to the client.
The molmo_spaces adapter's local `resize_with_pad` copy duplicated both the
server codec's resize AND the client's negotiated downsizing (`image_sizes`
session meta), risking silent eval-input drift; it is removed and frames now
pass through at native resolution. Prompt lowercasing moves from the adapter
into the DROID codec (`lowercase_task` on `ObservationCodec`, enabled in
`droid_obs`) so every client of those checkpoints gets it.

Documented the separation of responsibilities in offboard/README.md, with a
pointer from CLAUDE.md so adapter authors find it before writing one.

Requested-by: Vladimir Yakunin
Ticket: Positronic-Robotics/internal#91 #refs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3cad3600c

ℹ️ 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".

Comment thread positronic/simulator/molmo_spaces/adapter.py Outdated
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

MolmoSpaces' learned-policy configs carry `remote_config: dict(host, port)`
for remotely served policies, so the normal `policy_factory(exp_config,
task)` path must connect to the configured endpoint — a default client no
longer silently lands on `localhost:8000`. Explicit `client`/`client_kwargs`
still win.

Ticket: none (Codex review finding on PR #495)
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ad3450e63

ℹ️ 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".

Comment thread pyproject.toml
The workflow overrides testpaths with a hard-coded core list, so the new
suite wasn't exercised on PRs despite its pyproject testpaths entry.

Ticket: none (Codex review finding on PR #495)
@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 34db8ead4d

ℹ️ 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".

@vertix

vertix commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing unmerged — superseded by #504 (the MolmoSpaces env-server integration), which realizes the loop-ownership invariant (#503). The served-policy shape here handed a positronic policy to MolmoSpaces' own loop, inverting control — which the invariant rejects (positronic always owns the loop).

Everything relevant from this PR is carried into #504: the two pure mapping functions (re-pointed to the env-server direction), the constants, the droid_obs fixture + mapping tests, and all of this PR's Codex fixes — camera-variant resolution (extended with the RandCam exterior randomized_zed2_analogue_1), prompt lowercasing (moved into ObservationCodec.lowercase_task, now applied to training + inference), and the CI testpaths + the adapter/codec/wire-client separation doc. #504 is additionally smoke-validated end-to-end on CPU.

@vertix vertix closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants