feat: Add protocol-driven GR00T inference compatibility#5
Conversation
21b9126 to
bcbd518
Compare
Adapt the GR00T policy client to work with both sim_wrapper (Isaac-GR00T N1.6+ Gr00tSimPolicyWrapper) and direct (N1.5) server protocols. Key design: data configs are UNCHANGED — they describe WHAT keys to use. A new PROTOCOLS registry describes HOW to format and send observations (video shape, state dtype, request wrapping). Protocol is resolved from the config or overridden at runtime. Changes: - data_config.py: Add PROTOCOLS registry and protocol field to libero configs. All original configs preserved exactly as-is. - __init__.py: Single _build_observation() adapts video dims, state dtype, language format based on active protocol. Flexible action parsing handles both response formats. - client.py: Protocol-aware request wrapping with auto-detection. - tests/test_groot_n1d6.py: 26 tests covering config integrity, observation formatting, action conversion, protocol selection. Tested with real GR00T N1.6 server (A10G GPU): 3/3 Libero spatial tasks completed (reward=1.0)
bcbd518 to
7a3e33e
Compare
|
@cagataycali Hey! Could you review this PR when you get a chance? 🙏 |
yinsong1986
left a comment
There was a problem hiding this comment.
PR #5 — feat: Add protocol-driven GR00T inference compatibility
Summary: Adds a two-protocol dispatch layer (sim_wrapper / direct) to the GR00T policy client so it can communicate with both Isaac-GR00T N1.5 and N1.6 servers without changing data configs.
🟡 Suggestions (non-blocking)
-
[
__init__.py/_map_libero_state] The original code had an explicitelsebranch that zeroed out all state keys whenrobot0_eef_pos/robot0_eef_quatwere missing. The new version drops it — if those keys are absent, state entries are never added toobs. Worth restoring as a defensive fallback. -
[
client.py/get_actionauto-detect]_requestnever raises on a malformed response, so theexcept RuntimeErrorfallback todirectwill never trigger in practice. Consider validating the response shape/structure and falling back on that instead. -
[
__init__.py/_create_fallback_actions] Fallback now returnsnp.zeros((h, 1))for every key regardless of actual action dimensionality. Worth restoring per-key dim inference from key names (joint_pos→7,eef_pos→3, etc.). -
[
__init__.py/_add_video_dims]reshape(1, 1, *image.shape)assumes input is always(H, W, C). Anassert image.ndim == 3before the reshape would catch edge cases early. -
[
data_config.py/_lookup] Fuzzy libero match +protocoloverride precedence is implicit — a short comment would help future readers. -
[
tests/test_groot_n1d6.py] Consider mockingGR00TClientin tests that instantiateGr00tPolicyto make isolation explicit.
✅ Verdict
Needs changes (minor) — solid, well-structured work with good test coverage and clean backward compatibility. A few small defensive gaps worth addressing.
- _map_libero_state: zero-fill state when eef_pos/eef_quat missing - client.py auto-detect: validate response structure via _has_action_keys - _create_fallback_actions: infer per-key dims from key names - _add_video_dims: assert image.ndim == 3 for early error detection - _resize_image: ensure 3-D output in exception fallback path - _lookup: add docstring with resolution order and override semantics - tests: patch GR00TClient to avoid real ZMQ sockets, add 5 new tests 35/35 tests passing.
8b410e6 to
4bc4063
Compare
FYI: @eraykeskinmac failing the lint test. Thanks! |
Summary
Add multi-protocol support for the GR00T policy client, enabling communication with both
Gr00tSimPolicyWrapperservers (Isaac-GR00T N1.6+) and direct policy servers (N1.5).Problem
Isaac-GR00T N1.6 changed the server-side inference protocol: video observations require a temporal dimension
(B,T,H,W,C), state values must befloat32, requests must be wrapped in{"observation": obs}, and action responses include a batch dimension. The existing client only supports the older format, causing inference failures with newer servers.Approach
Data configs are unchanged. The original key mappings (video, state, action, language) work with both protocols. Only the transport-level formatting differs — this is handled by a new
PROTOCOLSregistry:sim_wrapper(B,T,H,W,C){"observation": obs}(B,T,dim)direct(B,H,W,C)(T,dim)Adding future protocols requires only a new entry in
PROTOCOLS— no changes toGr00tPolicyor data configs.Changes
data_config.pyPROTOCOLSregistry. Addprotocolfield to libero configs. Original config keys untouched.__init__.py_build_observation()adapts to protocol. Batch dim stripping for responses.client.pytests/test_groot_n1d6.pyBackward compatibility
test_original_configs_unchanged)groot_version="n1d6"kwarg still works (maps toprotocol="sim_wrapper")protocolfield default to auto-detectionUsage
Test results
black,isort,flake8)0xAnkitSingh/GR00T-N1.6-LIBEROcheckpoint