feat(phyai):Adds GR00T-N1.7 model support#41
Conversation
📝 WalkthroughWalkthroughThis PR adds GR00T-N1.7 support across preprocessing, checkpoint-agnostic processing, Qwen3-VL integration, model/configuration code, CUDA-graph runners, scheduler/plugin wiring, and supporting tooling updates. ChangesGR00T-N1.7 native inference implementation
Tooling and packaging updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Engine
participant GR00TN17Entry
participant GR00TN17WS1Scheduler
participant GR00TN17BackboneRunner
participant GR00TN17ActionHeadRunner
Engine->>GR00TN17Entry: setup(args)
GR00TN17Entry->>GR00TN17WS1Scheduler: setup()
GR00TN17WS1Scheduler->>GR00TN17BackboneRunner: setup()
GR00TN17WS1Scheduler->>GR00TN17ActionHeadRunner: setup()
Engine->>GR00TN17Entry: step(request)
GR00TN17Entry->>GR00TN17WS1Scheduler: step(request)
GR00TN17WS1Scheduler->>GR00TN17BackboneRunner: forward(inputs)
GR00TN17BackboneRunner-->>GR00TN17WS1Scheduler: GR00TN17BackboneOutput
GR00TN17WS1Scheduler->>GR00TN17ActionHeadRunner: forward(backbone_output, action_input, noise)
GR00TN17ActionHeadRunner-->>GR00TN17WS1Scheduler: normalized action tensor
GR00TN17WS1Scheduler-->>GR00TN17Entry: normalized action tensor
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces native inference support for the GR00T-N1.7 model, adding checkpoint-agnostic preprocessing utilities in phyai-utils-tools and engine integration components (model, runner, scheduler, and configs) in phyai. Feedback on the changes highlights a bug in _process_vlm_injected where batched image inputs are incorrectly flattened, and another bug in _build_norm_params where relative action statistics fail to respect the use_percentiles flag. Additionally, performance optimization opportunities were identified, such as performing video frame stacking and reshaping entirely in NumPy to avoid PyTorch CPU overhead, and replacing a manual layer normalization implementation with PyTorch's optimized F.layer_norm.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| all_images: list[Image.Image] = [] | ||
| for images_for_item, language in zip(transformed_images, languages): | ||
| pil_images = [ | ||
| Image.fromarray(np.transpose(v, (1, 2, 0))) for v in images_for_item | ||
| ] | ||
| conversation = [ | ||
| { | ||
| "role": "user", | ||
| "content": [ | ||
| *[{"type": "image", "image": img} for img in pil_images], | ||
| {"type": "text", "text": language}, | ||
| ], | ||
| } | ||
| ] | ||
| text = processor.apply_chat_template( | ||
| conversation, tokenize=False, add_generation_prompt=False | ||
| ) | ||
| texts.append(text) | ||
| all_images.extend(pil_images) | ||
| tokenized = processor( | ||
| text=texts, | ||
| images=all_images, | ||
| return_tensors="pt", | ||
| padding=True, | ||
| ) |
There was a problem hiding this comment.
In _process_vlm_injected, all_images is populated as a flat list of images across all batch items using all_images.extend(pil_images). However, when len(texts) > 1 (batched input), the HuggingFace Qwen2VLProcessor expects images to be a list of lists of images (one list per text prompt). Passing a flat list of images when there are multiple text prompts will cause a ValueError due to a length mismatch. Change all_images to be a list of lists of images by appending pil_images instead of extending.
all_images: list[list[Image.Image]] = []
for images_for_item, language in zip(transformed_images, languages):
pil_images = [
Image.fromarray(np.transpose(v, (1, 2, 0))) for v in images_for_item
]
conversation = [
{
"role": "user",
"content": [
*[{"type": "image", "image": img} for img in pil_images],
{"type": "text", "text": language},
],
}
]
text = processor.apply_chat_template(
conversation, tokenize=False, add_generation_prompt=False
)
texts.append(text)
all_images.append(pil_images)
tokenized = processor(
text=texts,
images=all_images,
return_tensors="pt",
padding=True,
)| norm_params[embodiment_tag]["action"][key] = { | ||
| k: np.asarray(v, dtype=np.float32) | ||
| for k, v in relative.items() | ||
| if k in ("min", "max", "mean", "std", "q01", "q99") | ||
| } | ||
| norm_params[embodiment_tag]["action"][key]["dim"] = action_dim |
There was a problem hiding this comment.
In _build_norm_params, when use_relative_action is enabled, the relative action statistics are copied directly from relative.items() without checking use_percentiles. This means "min" and "max" are set to the raw minimum and maximum values, even if use_percentiles is True. For normal actions and states, "min" and "max" are correctly mapped to "q01" and "q99" when use_percentiles is True. Update the relative action normalization parameters to respect use_percentiles as well.
min_vals = np.asarray(
relative["q01"] if use_percentiles and "q01" in relative else relative["min"],
dtype=np.float32,
)
max_vals = np.asarray(
relative["q99"] if use_percentiles and "q99" in relative else relative["max"],
dtype=np.float32,
)
norm_params[embodiment_tag]["action"][key] = {
"min": min_vals,
"max": max_vals,
"mean": np.asarray(relative["mean"], dtype=np.float32),
"std": np.asarray(relative["std"], dtype=np.float32),
"dim": action_dim,
}| images = [torch.from_numpy(observation.video[key]) for key in image_keys] | ||
| stacked = torch.stack(images, dim=2) | ||
| if stacked.ndim != 6: | ||
| raise ValueError("stacked video must have shape (B,T,V,H,W,C).") | ||
| batch, time, views, height, width, channels = stacked.shape | ||
| images_flat = stacked.reshape(batch, time * views, height, width, channels) | ||
| transformed_images = [] | ||
| for b in range(batch): | ||
| transformed_frames = [] | ||
| for frame in images_flat[b]: | ||
| transformed_frames.append( | ||
| eval_transform_image( | ||
| frame.numpy(), | ||
| shortest_image_edge=self.shortest_image_edge, | ||
| image_target_size=self.image_target_size, | ||
| image_crop_size=self.image_crop_size, | ||
| crop_fraction=self.crop_fraction, | ||
| ) | ||
| ) | ||
| transformed_images.append(np.stack(transformed_frames, axis=0)) |
There was a problem hiding this comment.
In _process_vlm, the video frames are converted to PyTorch tensors, stacked, reshaped, and then converted back to numpy arrays via frame.numpy() for image transformation. Since observation.video[key] is already a numpy array, we can perform the stacking and reshaping entirely in numpy. This avoids the unnecessary overhead of PyTorch tensor creation, stacking, and reshaping on the CPU.
| images = [torch.from_numpy(observation.video[key]) for key in image_keys] | |
| stacked = torch.stack(images, dim=2) | |
| if stacked.ndim != 6: | |
| raise ValueError("stacked video must have shape (B,T,V,H,W,C).") | |
| batch, time, views, height, width, channels = stacked.shape | |
| images_flat = stacked.reshape(batch, time * views, height, width, channels) | |
| transformed_images = [] | |
| for b in range(batch): | |
| transformed_frames = [] | |
| for frame in images_flat[b]: | |
| transformed_frames.append( | |
| eval_transform_image( | |
| frame.numpy(), | |
| shortest_image_edge=self.shortest_image_edge, | |
| image_target_size=self.image_target_size, | |
| image_crop_size=self.image_crop_size, | |
| crop_fraction=self.crop_fraction, | |
| ) | |
| ) | |
| transformed_images.append(np.stack(transformed_frames, axis=0)) | |
| images = [observation.video[key] for key in image_keys] | |
| stacked = np.stack(images, axis=2) | |
| if stacked.ndim != 6: | |
| raise ValueError("stacked video must have shape (B,T,V,H,W,C).") | |
| batch, time, views, height, width, channels = stacked.shape | |
| images_flat = stacked.reshape(batch, time * views, height, width, channels) | |
| transformed_images = [] | |
| for b in range(batch): | |
| transformed_frames = [] | |
| for frame in images_flat[b]: | |
| transformed_frames.append( | |
| eval_transform_image( | |
| frame, | |
| shortest_image_edge=self.shortest_image_edge, | |
| image_target_size=self.image_target_size, | |
| image_crop_size=self.image_crop_size, | |
| crop_fraction=self.crop_fraction, | |
| ) | |
| ) | |
| transformed_images.append(np.stack(transformed_frames, axis=0)) |
| x_float = x.float() | ||
| mean = x_float.mean(dim=-1, keepdim=True) | ||
| var = (x_float - mean).square().mean(dim=-1, keepdim=True) | ||
| out = (x_float - mean) * torch.rsqrt(var + eps) | ||
| out = out.to(dtype=x.dtype) | ||
| if weight is not None: | ||
| out = out * weight.to(device=x.device, dtype=x.dtype) | ||
| if bias is not None: | ||
| out = out + bias.to(device=x.device, dtype=x.dtype) | ||
| return out |
There was a problem hiding this comment.
The manual implementation of layer normalization in _torch_layer_norm performs multiple passes over the last dimension of the tensor, which is inefficient and uses unnecessary temporary memory. PyTorch provides a highly optimized F.layer_norm operator that is faster, more memory-efficient, and numerically stable on both CPU and GPU. Use F.layer_norm instead.
| x_float = x.float() | |
| mean = x_float.mean(dim=-1, keepdim=True) | |
| var = (x_float - mean).square().mean(dim=-1, keepdim=True) | |
| out = (x_float - mean) * torch.rsqrt(var + eps) | |
| out = out.to(dtype=x.dtype) | |
| if weight is not None: | |
| out = out * weight.to(device=x.device, dtype=x.dtype) | |
| if bias is not None: | |
| out = out + bias.to(device=x.device, dtype=x.dtype) | |
| return out | |
| return F.layer_norm(x, (x.shape[-1],), weight, bias, eps) |
yuerqiqi
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
phyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.py (1)
348-350: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead assignments;
statistics/embodiment_id_mappingoverrides are silently ignored.
kwargs["statistics"]andkwargs["embodiment_id_mapping"]are set but excluded fromkeep, so they never reachinit_kwargs; both values are instead passed explicitly at Lines 378-379. This means a caller-suppliedstatistics/embodiment_id_mappinginoverridesis dropped. Remove the redundant assignments or fold these into the override flow if overriding is intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.py` around lines 348 - 350, The redundant assignments in the GR00T processor setup cause caller-provided statistics and embodiment_id_mapping overrides to be ignored. Update the initialization flow in processor_gr00t.py around the init_kwargs/overrides handling so these values are either included in the preserved kwargs path or removed from the separate explicit assignment path, ensuring overrides passed into the GR00T processor are not silently dropped.phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py (1)
43-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate layer-truncation logic across two files.
This method mirrors
GR00TN17Backbone._truncate_language_layersinmodeling_gr00t_n17.py(lines 248-259), which independently re-deriveslanguage_modeland re-implements the same "pop from end while over budget" loop. Consider extracting a shared helper (e.g., in a small util module) taking alanguage_model.layers-likeModuleListandselect_layerto avoid keeping two divergence-prone copies in sync.♻️ Example shared helper
def truncate_language_layers(layers: nn.ModuleList, select_layer: int) -> None: if select_layer < 0: return while len(layers) > select_layer: layers.pop(-1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py` around lines 43 - 48, The layer truncation logic is duplicated between qwen3_vl_adapter._truncate_language_layers and GR00TN17Backbone._truncate_language_layers, so extract the shared “truncate from end while over select_layer” behavior into a common helper and have both call it. Use the existing language_model.layers ModuleList and select_layer parameter in the helper, and update both _truncate_language_layers methods to delegate instead of re-implementing the loop.phyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.py (2)
460-542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
torch.nn.initover hand-rolled initializers.
_calculate_fan_in_and_fan_out,_init_kaiming_uniform_,_init_uniform_,_init_normal_, and_init_zeros_re-implementtorch.nn.init.kaiming_uniform_/uniform_/normal_/zeros_. The math matches PyTorch's implementation, but maintaining a parallel copy of well-tested initializer formulas is unnecessary risk for future edits.♻️ Example simplification
-def _init_kaiming_uniform_(tensor: torch.Tensor, a: float) -> None: - fan_in, _ = _calculate_fan_in_and_fan_out(tensor) - gain = math.sqrt(2.0 / (1.0 + a**2)) - std = gain / math.sqrt(fan_in) - bound = math.sqrt(3.0) * std - with torch.no_grad(): - tensor.uniform_(-bound, bound) +def _init_kaiming_uniform_(tensor: torch.Tensor, a: float) -> None: + nn.init.kaiming_uniform_(tensor, a=a)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.py` around lines 460 - 542, The custom initializer helpers in GR00TN17Linear duplicate torch.nn.init behavior and should be replaced with the standard PyTorch init utilities. Update the parameter setup in GR00TN17Linear.reset_parameters to use torch.nn.init for kaiming_uniform_, uniform_, normal_, and zeros_ instead of the local helpers, and remove the redundant helper functions such as _calculate_fan_in_and_fan_out, _init_kaiming_uniform_, _init_uniform_, _init_normal_, and _init_zeros_ if they are no longer used.
265-292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
vision_attention_backendisn't wired to config; always resolves to the adapter's hardcoded default.
_build_qwen3vl_modelpassesattention_backend=self.config.attention_backendbut never avision_attention_backend, so the vision backbone always usesGR00TN17Qwen3VLBackbone's default ("flashinfer") regardless ofGR00TN17BackboneConfig. If this is intentional (vision always flashinfer), consider a brief comment; otherwise expose a config knob so it's consistent with howattention_backendis already configurable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.py` around lines 265 - 292, The vision attention backend is not configurable in _build_qwen3vl_model, so GR00TN17Qwen3VLBackbone always falls back to its default instead of using GR00TN17BackboneConfig. Update the GR00TN17 configuration and the _build_qwen3vl_model path to pass an explicit vision_attention_backend through to GR00TN17Qwen3VLBackbone, similar to how attention_backend is already wired, or add a short comment if the default is intentionally fixed.phyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.py (1)
265-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftFast path duplicates and tightly couples to Qwen3-VL internals.
_backbone_corere-derives the full ViT→merger→deepstack→LLM pipeline by reaching intoqwen_model.visual/qwen_model.language_modelinternals instead of the model's own forward. This is understandable for CUDA-graph capture, but it means any future change to the shared Qwen3-VL vision/LLM forward (e.g., inmodeling_qwen3_vl.py) must be manually mirrored here or the graph path will silently diverge from the eager path. Consider adding a regression/parity test asserting graph-path vs eager-path output equivalence, and/or a comment pointing back to the source-of-truth forward method(s) it must stay in sync with.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.py` around lines 265 - 352, The `_backbone_core` fast path is duplicating Qwen3-VL’s vision/LLM forward logic and can drift from the eager path. Add a parity/regression test that compares this path against the source-of-truth forward behavior, and add a clear sync note near `_backbone_core` referencing the `qwen_model.visual` and `qwen_model.language_model` flow so future changes in the shared forward implementation are mirrored here.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@phyai-utils-tools/src/phyai_utils_tools/models/gr00t/ops_gr00t.py`:
- Around line 149-173: The GR00T eval transform in op_gr00t.py has a silent PIL
fallback that can produce different pixels than the cv2 path, so make the
runtime dependency explicit or document the behavior. Update the package
manifest to include opencv-python for the image transform path used by
_eval_transform_image, or if the fallback must remain, clearly mark
_eval_transform_image_cv2 and the PIL branch as non-identical so users know eval
results may vary across environments.
In `@phyai/src/phyai/models/gr00t_n17/main_gr00t_n17.py`:
- Around line 137-144: Remove the dead no-op call to the private method on the
backbone before loading weights. In the checkpoint-loading branch of
main_gr00t_n17.py, delete the self.model.backbone._load_qwen3vl_model() call and
keep only the load_pretrained flow, unless you intended a distinct pre-load
step; if so, replace it with an explicit public action instead of reaching into
the _load_qwen3vl_model helper.
In `@phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py`:
- Around line 22-41: The Qwen3VL adapter constructor is hard-coding the text
attention backend and bypassing shared backend resolution. Update
Qwen3VLAdapter.__init__ so it does not force attention_backend="sdpa"; instead
pass through None or the engine-configured value when calling the
Qwen3VLForConditionalGeneration parent init, while keeping the vision backend
behavior unchanged.
---
Nitpick comments:
In `@phyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.py`:
- Around line 348-350: The redundant assignments in the GR00T processor setup
cause caller-provided statistics and embodiment_id_mapping overrides to be
ignored. Update the initialization flow in processor_gr00t.py around the
init_kwargs/overrides handling so these values are either included in the
preserved kwargs path or removed from the separate explicit assignment path,
ensuring overrides passed into the GR00T processor are not silently dropped.
In `@phyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.py`:
- Around line 265-352: The `_backbone_core` fast path is duplicating Qwen3-VL’s
vision/LLM forward logic and can drift from the eager path. Add a
parity/regression test that compares this path against the source-of-truth
forward behavior, and add a clear sync note near `_backbone_core` referencing
the `qwen_model.visual` and `qwen_model.language_model` flow so future changes
in the shared forward implementation are mirrored here.
In `@phyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.py`:
- Around line 460-542: The custom initializer helpers in GR00TN17Linear
duplicate torch.nn.init behavior and should be replaced with the standard
PyTorch init utilities. Update the parameter setup in
GR00TN17Linear.reset_parameters to use torch.nn.init for kaiming_uniform_,
uniform_, normal_, and zeros_ instead of the local helpers, and remove the
redundant helper functions such as _calculate_fan_in_and_fan_out,
_init_kaiming_uniform_, _init_uniform_, _init_normal_, and _init_zeros_ if they
are no longer used.
- Around line 265-292: The vision attention backend is not configurable in
_build_qwen3vl_model, so GR00TN17Qwen3VLBackbone always falls back to its
default instead of using GR00TN17BackboneConfig. Update the GR00TN17
configuration and the _build_qwen3vl_model path to pass an explicit
vision_attention_backend through to GR00TN17Qwen3VLBackbone, similar to how
attention_backend is already wired, or add a short comment if the default is
intentionally fixed.
In `@phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py`:
- Around line 43-48: The layer truncation logic is duplicated between
qwen3_vl_adapter._truncate_language_layers and
GR00TN17Backbone._truncate_language_layers, so extract the shared “truncate from
end while over select_layer” behavior into a common helper and have both call
it. Use the existing language_model.layers ModuleList and select_layer parameter
in the helper, and update both _truncate_language_layers methods to delegate
instead of re-implementing the loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6b50832-b38d-4dde-a3b4-0dc39024de16
📒 Files selected for processing (14)
CLAUDE.mdphyai-utils-tools/pyproject.tomlphyai-utils-tools/src/phyai_utils_tools/models/gr00t/__init__.pyphyai-utils-tools/src/phyai_utils_tools/models/gr00t/ops_gr00t.pyphyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.pyphyai/src/phyai/engine.pyphyai/src/phyai/models/gr00t_n17/__init__.pyphyai/src/phyai/models/gr00t_n17/configuration_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/main_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.pyphyai/src/phyai/models/gr00t_n17/scheduler_ws1_gr00t_n17.pyphyai/src/phyai/models/qwen3_vl/modeling_qwen3_vl.py
| def __init__( | ||
| self, | ||
| config: Qwen3VLConfig, | ||
| *, | ||
| select_layer: int, | ||
| params_dtype: torch.dtype | None = None, | ||
| device: torch.device | str | None = None, | ||
| attention_backend: str = "sdpa", | ||
| vision_attention_backend: str = "flashinfer", | ||
| ) -> None: | ||
| super().__init__( | ||
| config, | ||
| params_dtype=params_dtype, | ||
| device=device, | ||
| attn_backend=attention_backend, | ||
| vision_attn_backend=vision_attention_backend, | ||
| prefix="backbone.model", | ||
| ) | ||
| self.select_layer = int(select_layer) | ||
| self._truncate_language_layers() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "MagiAttention|resolve_attn_backend|\bCP\b" phyai/src/phyai/layers -C3
rg -n "attn_backend|vision_attn_backend" phyai/src/phyai/models/qwen3_vl/modeling_qwen3_vl.py -C3Repository: MEmbodied/phyai
Length of output: 5628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
ast-grep outline phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py --view expanded
ast-grep outline phyai/src/phyai/models/qwen3_vl/modeling_qwen3_vl.py --view expanded
# Inspect the adapter and the parent constructor around backend wiring
sed -n '1,140p' phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py
printf '\n--- parent model ---\n'
sed -n '680,760p' phyai/src/phyai/models/qwen3_vl/modeling_qwen3_vl.py
printf '\n--- engine defaults ---\n'
rg -n "def resolve_engine_defaults|CP|MagiAttention|flashinfer|sdpa" phyai/src/phyai -C3Repository: MEmbodied/phyai
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the default-resolution path and CP/MagiAttention wiring
sed -n '608,660p' phyai/src/phyai/engine_config.py
printf '\n--- MagiAttention references ---\n'
rg -n "MagiAttention|cp_size|context parallel|attn_backend.*sdpa|vision_attention_backend" phyai/src/phyai -C3
printf '\n--- GR00T adapter usages ---\n'
rg -n "GR00TN17Qwen3VLBackbone|qwen3_vl_adapter" phyai/src/phyai -C3Repository: MEmbodied/phyai
Length of output: 10517
Stop pinning the text attention backend here
Qwen3VLForConditionalGeneration only applies engine defaults when an arg is None; passing attention_backend="sdpa" here bypasses the shared flashinfer default on CUDA and any CP-driven backend switch. Pass None (or thread through the engine config) so this adapter inherits the normal backend resolution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@phyai/src/phyai/models/gr00t_n17/qwen3_vl_adapter.py` around lines 22 - 41,
The Qwen3VL adapter constructor is hard-coding the text attention backend and
bypassing shared backend resolution. Update Qwen3VLAdapter.__init__ so it does
not force attention_backend="sdpa"; instead pass through None or the
engine-configured value when calling the Qwen3VLForConditionalGeneration parent
init, while keeping the vision backend behavior unchanged.
Source: Coding guidelines
yuerqiqi
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (2)
phyai-utils-tools/pyproject.toml (2)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVerify
fastokensversion constraint is intentional.
fastokens>=0.2.0uses an open-ended lower bound; confirm this is intentional versus pinning an exact/known-good version to avoid unexpected breakage from future releases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai-utils-tools/pyproject.toml` around lines 17 - 20, The fastokens dependency in the optional-dependencies section uses an open-ended minimum version, so confirm the intended release policy and update the fastokenizer requirement accordingly. If you want a stable known-good dependency, change the constraint to an exact or bounded version; otherwise keep the lower bound as-is, but make the choice explicit in pyproject.toml near fastokenizer.
10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider pinning new dependency versions.
New deps (
opencv-python-headless,pillow,torchvision) appear to be added without explicit version pins, unliketorch==2.11andtransformers==5.8.1in the same list. For reproducible builds, especially withtorchvisionwhich must stay compatible with the pinnedtorch==2.11, consider adding explicit or range-bounded pins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phyai-utils-tools/pyproject.toml` around lines 10 - 15, The dependency list in pyproject.toml adds opencv-python-headless, pillow, and torchvision without version constraints, which can break reproducible builds and torch compatibility. Update the dependency entries in the same list to use explicit pins or safe version ranges, and make sure torchvision is constrained to a version compatible with the existing torch==2.11 pin. Keep the existing style consistent with the other pinned dependencies like torch and transformers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@phyai-utils-tools/pyproject.toml`:
- Around line 17-20: The fastokens dependency in the optional-dependencies
section uses an open-ended minimum version, so confirm the intended release
policy and update the fastokenizer requirement accordingly. If you want a stable
known-good dependency, change the constraint to an exact or bounded version;
otherwise keep the lower bound as-is, but make the choice explicit in
pyproject.toml near fastokenizer.
- Around line 10-15: The dependency list in pyproject.toml adds
opencv-python-headless, pillow, and torchvision without version constraints,
which can break reproducible builds and torch compatibility. Update the
dependency entries in the same list to use explicit pins or safe version ranges,
and make sure torchvision is constrained to a version compatible with the
existing torch==2.11 pin. Keep the existing style consistent with the other
pinned dependencies like torch and transformers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a271b9f-9ed0-40d7-a172-16e2112f7efa
📒 Files selected for processing (9)
phyai-utils-tools/pyproject.tomlphyai-utils-tools/src/phyai_utils_tools/models/gr00t/ops_gr00t.pyphyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.pyphyai/src/phyai/models/gr00t_n17/__init__.pyphyai/src/phyai/models/gr00t_n17/configuration_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/main_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/modeling_gr00t_n17.pyphyai/src/phyai/models/gr00t_n17/scheduler_ws1_gr00t_n17.py
💤 Files with no reviewable changes (2)
- phyai/src/phyai/models/gr00t_n17/init.py
- phyai/src/phyai/models/gr00t_n17/main_gr00t_n17.py
🚧 Files skipped from review as they are similar to previous changes (3)
- phyai/src/phyai/models/gr00t_n17/scheduler_ws1_gr00t_n17.py
- phyai-utils-tools/src/phyai_utils_tools/models/gr00t/processor_gr00t.py
- phyai/src/phyai/models/gr00t_n17/model_runner_gr00t_n17.py
1 participant
Summary
Adds GR00T-N1.7 model support to PhyAI.
This PR includes:
phyai-utils-toolsNotes
qwen3_vl_native.pyimplementation withqwen3_vl_adapter.py, which adapts the shared Qwen3-VL implementation.Summary by CodeRabbit
fastokenizerextra, and expanded core dependencies for image and vision processing.