Skip to content

Unify ONNX deployment dependencies under policy-deploy - #624

Open
acrlw wants to merge 1 commit into
mainfrom
enhance/policy-deploy-extra
Open

acrlw wants to merge 1 commit into
mainfrom
enhance/policy-deploy-extra

Conversation

@acrlw

@acrlw acrlw commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Description

Replace the NMG-specific nmg extra, which installs CPU ONNX Runtime, with
policy-deploy for both NeuralPlanner and ONNX-backed DexSim Motion Policy Kit
consumers. pip install "embodichain[policy-deploy]" installs
onnxruntime-gpu>=1.20,<1.27; base dependencies remain unchanged.

The upper bound keeps the documented CUDA 12 stack compatible: ORT 1.27+
defaults to CUDA 13 according to the
official compatibility table.
Update NeuralPlanner's missing-runtime hint, installation instructions, and
motion-planning context to use the shared extra.

Migration: replace [nmg] with [policy-deploy] in install commands. Existing
CPU ORT installations need removal before installing the GPU package; the
installation guide includes cleanup instructions for environments with both
packages. NMG functionality and execution-provider selection are preserved.

Related: #520.

Type of change

  • Breaking change (existing [nmg] installation commands require migration)

Validation

  • Built sdist and wheel with uv build --no-sources; both pass twine check.
  • Checked both artifacts' metadata: base requirements unchanged, nmg absent,
    and only policy-deploy selects onnxruntime-gpu>=1.20,<1.27.
  • Installed the wheel with --no-deps in an isolated environment and verified
    installed extra metadata and the packaged NeuralPlanner installation hint.
  • Black 26.3.1 repository check passes (1,032 files); API docs coverage passes
    (2,067/2,067 exports); context checker and 23 context tests pass.
  • Changed MyST pages parse successfully and their local Markdown links resolve.

Validation covers packaging and documentation. Full simulation/GPU inference
tests and a full Sphinx build were not run. Black emitted a Python-version
advisory, and pytest emitted a cache-directory warning; both checks exited 0.

Screenshots

Not applicable.

Checklist

  • I have run the black . command to format the code base.
  • I have made corresponding changes to the documentation.
  • Public API documentation coverage check passes; no public API signatures changed.
  • I have added tests that prove my fix is effective or that my feature works. No new runtime behavior; built-artifact checks and existing context tests were used.
  • Dependencies have been updated.

Share GPU ONNX Runtime across NeuralPlanner and ONNX policy deployment, constrain it to the CUDA 12 release line, and document migration from the CPU-only nmg extra.

BREAKING CHANGE: replace embodichain[nmg] with embodichain[policy-deploy] and remove any existing CPU ONNX Runtime package before installing the GPU runtime.
@acrlw acrlw added breaking dependencies Pull requests that update a dependency file motion gen Things related to motion generation for robot rl Features related to reinforcement learning labels Sep 14, 2026
@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking gap in automated coverage for its new packaging contract.

Fix All in CodexFindings

  1. P2 Packaging Contract Lacks Coverage
Fix with agent prompt
### Issue 1
pyproject.toml:54-58
The new public installation contract is only manually validated. Existing metadata tests iterate through extras but do not assert that `policy-deploy` exists, `nmg` is absent, or the CUDA-compatible `<1.27` bound remains. A focused metadata test would prevent future packaging changes from silently breaking the documented migration or CUDA 12 compatibility.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

  • Replaces CPU onnxruntime with bounded onnxruntime-gpu>=1.20,<1.27.
  • Updates NeuralPlanner’s missing-runtime message and user-facing installation documentation.
  • Documents CUDA compatibility, package cleanup, and migration instructions.
  • Leaves the base dependency set and planner execution-provider selection unchanged.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Extra[policy-deploy extra] --> ORT[onnxruntime-gpu 1.20 to 1.26]
  ORT --> Neural[NeuralPlanner / NMG]
  ORT --> MotionKit[DexSim Motion Policy Kit]
  Providers[Configured execution providers] --> Neural
  Providers --> MotionKit
Loading

Reviews (1) · Last reviewed commit: "build(deps)!: replace nmg extra with pol..."

Comment thread pyproject.toml
Comment on lines 54 to 58
[project.optional-dependencies]
nmg = [
"onnxruntime>=1.20",
policy-deploy = [
# ORT 1.27+ defaults to CUDA 13; EmbodiChain's supported stack uses CUDA 12.
"onnxruntime-gpu>=1.20,<1.27",
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Packaging Contract Lacks Coverage

The new public installation contract is only manually validated. Existing metadata tests iterate through extras but do not assert that policy-deploy exists, nmg is absent, or the CUDA-compatible <1.27 bound remains. A focused metadata test would prevent future packaging changes from silently breaking the documented migration or CUDA 12 compatibility.

Prompt To Fix With AI
This is a comment left during a code review.
Path: pyproject.toml
Line: 54-58

Comment:
**Packaging Contract Lacks Coverage**

The new public installation contract is only manually validated. Existing metadata tests iterate through extras but do not assert that `policy-deploy` exists, `nmg` is absent, or the CUDA-compatible `<1.27` bound remains. A focused metadata test would prevent future packaging changes from silently breaking the documented migration or CUDA 12 compatibility.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking dependencies Pull requests that update a dependency file motion gen Things related to motion generation for robot rl Features related to reinforcement learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants