Skip to content

Simplify Task Program configuration and repair task deployments - #1194

Open
yuecideng wants to merge 5 commits into
mainfrom
codex/task-program-config-cleanup
Open

yuecideng wants to merge 5 commits into
mainfrom
codex/task-program-config-cleanup

Conversation

@yuecideng

@yuecideng yuecideng commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

Task Program deployments repeat logical/native identifiers, fixed-target scaffolding and long transform matrices. This PR simplifies authoring while retaining legacy input forms, explicit physical bindings and the existing acceptance contracts. It is isolated from #1175 and based on main.

  • Omit same-name scene simulation_uid values, use fixed pose targets, accept compact position plus XYZW quaternion or explicit rotation matrix, and use canonical skill_options. Legacy flat matrices and former aliases remain supported. Rounded calibration matrices retain their coefficients without orthogonalization.
  • Allow bounded deployment integration overrides for declared resource defaults and allowlisted skill fields; keep scene, runtime-service and acceptance ownership closed.
  • Explain the composed deployment, physical backend, resource bindings, effective services and parameter sources with inspect_deployment.py --explain --json, without constructing simulation.
  • Retain the task repairs and opt-in Cartesian lift/approach and handover retraction. Preserve the original cloth stream (991 frames / 990 actions), without the additional two-second hold.
  • Install the pinned cloth robot/mesh bundle with hash verification and protection of existing local variants.

No new runtime dependencies.

Review fixes

Review-fix commit: 2ef586a5209b176fe02e046e2835a51f173698fa.

All eight findings against 253ae52fb are addressed:

Finding Resulting behavior
Optional native UID Normal decoding, asset materialization and pouring share the same strict resolver. Omission selects entity_id; explicit different mappings work; explicit invalid values still fail.
Named DemoSkill inspection Explain uses owned production-composed declarations after named-skill expansion. Actual bound occurrences remain visible in nominal_sources.
Positional API compatibility The new HandOver retraction and Place Cartesian fields are keyword-only; old positional constructors retain their original meaning.
Required Place arrivals Each authored approach leg is independently retimed. Exact endpoints and a one-control-interval stop preserve waypoint arrivals and acceleration bounds through concatenation.
YAML aliases Overrides copy and replace each selected defaults/options entry, isolating unselected siblings and the source.
Carton examples Both examples use production target/pose decoders and emit JSON-compatible pose declarations or flat 16-number constraints.
Recovery provenance Normalized recovery fields point to the original authored alias and deployment/inline/component location.
Interrupted installation Inputs are copied and verified in destination-directory temporary files, then atomically published without overwriting local files. Retry can recover.

The other four failures from the previous CI job are corrected at their test boundary: the policy inspector consumes a real invalid deployment, target checks use decoded poses, the repaired stacking deployment expects its 220-sample policy, and the shared-embodiment check verifies its declared placement override. Existing rejection and ownership assertions are retained.

Validation

  • 14,865 passed, 55 skipped, 0 failed in the same fast lane selected by PR CI, with eight workers. The first local run's five environment failures were reproduced on 253ae52fb; isolating the user's model-service configuration, generating the standard VERSION resource and exposing the existing DexSim installation to the temporary wheel consumer resolved them without test or production changes.
  • 37/37 before/after deployment snapshots match exactly: integration identity, full registration payload, compiled program identity, physical configuration and evaluation declarations. No official task YAML or acceptance criteria change in this review-fix commit.
  • Focused regression coverage includes omitted/equal/remapped/invalid UIDs, pouring support lookup, YAML alias isolation, positional compatibility, real adaptive retiming on the final command grid at 40/20 ms, partial failure, named DemoSkill explain, legacy recovery origins and interrupted URDF/mesh installation. Five new motion/API cases fail against the old production code and pass after the fix. Existing test modules were extended; no tutorial tests were added.
  • Both Carton preparation boundaries were exercised: actual present.prepare() plus explain succeeds, and the mixed example's decoded flat16 transform constructs its production constraint successfully.
  • 42 passed for project context; structural and canonical-skill checks pass. The owning configuration, motion and asset guidance was updated; other affected lifecycle/ownership guidance remains accurate.
  • black==26.10.1: black ., formatting check and git diff --check pass. Public API coverage: 4914/4914 exports documented.

Native verification and limits

Before the main synchronization, three independent normal-entry Marvin Repeated Pick/Place processes used seed 7, one environment, disabled sensors, task-owned Default/ConstraintDynamics physics and renderer, and the original three-attempt/4800-step budgets. All succeeded on their first attempt with every one of six execution segments accepted:

Run Steps Accepted segments
1 2464 6/6
2 2458 6/6
3 2469 6/6

Program/config hashes match across these runs. This is bounded fixed-seed verification, not a universal success-rate claim or complete attribution of prior grip-retention failures.

The cloth environment, original trajectory and cloth tests remain byte-identical to main; trajectory SHA-256 is 51f01325407ecc256c374d4d8c82a43222b90b10e69e7984daa620f1a346ab24. Cloth native simulation was not repeated in this review-fix round. Its earlier single-run final XY errors were 9.30 / 16.84 mm and remain rejected by the unchanged 15 mm rule. Other GPU/simulation CI lanes were not rerun locally.

Main synchronization

Merged latest main (b629c7158b8180934c0990b71e12e1031ddac16e) in b21c0f315fa4ad51f34ab5f3e7674bbd7f9c657c. The two PickUp/Place conflicts retain this PR's Cartesian branches and main's new success=motion_result.success propagation when splitting trajectories. No acceptance criteria were changed.

Post-merge verification: 14,860 passed, 55 skipped, all 37 deployment snapshots unchanged, formatting/context checks pass, and 4914/4914 public API exports documented. A fresh seed-7 normal-entry Marvin run also accepted all six segments in 2469 steps, with the original attempt and step budgets.

All eight original review threads now have individual replies describing the fix in 2ef586a52 and its validation. Threads are left for reviewer confirmation.

Follow-up review fixes

Commit 6a07480c739096140d036ee64c46ba7b999d3b16 addresses all five follow-up findings against b21c0f315:

  • Mask ineligible and cumulatively failed rows to current-FK hold targets before every Cartesian leg and retract, so rejected rows cannot exhaust the active batch timing budget.
  • Grow adaptive approach sampling to fit required arrivals, including sample_count=9; retain explicit fixed-grid budget validation.
  • Share the production integration selector resolver with asset requirement loading and deployment materialization, retaining bounded overrides and component-relative service paths.
  • Decode the Present example's static target independently of its source demonstration assets; preserve generated-deployment preflight. Strict offline preparation and explain make zero asset-resolution requests.
  • Decode all three Pour Water variants through production pose/transform decoders in the slow physical tests. All original physical assertions remain.

Fresh validation: 14,865 fast tests passed, 55 skipped; 682 focused tests passed; all 6 slow Pour Water cases passed using CI's SDL_VIDEODRIVER=dummy. The six physical runs retain the original success/retention/return gates, position error below 20 mm and tilt above 45 degrees. Measured maximum errors were 4.040 / 17.314 / 9.581 / 16.876 / 17.241 / 9.577 mm. All 37 deployment snapshots are unchanged; API coverage 4914/4914, context 42 passed, black 26.10.1 and diff checks pass. Independent reviews additionally checked an initially ineligible or previously failed 1000 m target against the healthy-row baseline, selector-relative manifests and unselected-object protection.

Because test_pour_water_sim.py is now directly modified, the existing CI planner selects it in slow-gpu for this PR; no selector rules, thresholds, task YAML or cloth replay were changed.

Type of change

  • Enhancement
  • Bug fix
  • Documentation update

Screenshots

Not applicable. Validation records include structured native outcomes and before/after deployment snapshots.

Checklist

  • I have run black . with black 26.10.1.
  • I reviewed affected context and updated its owning guidance.
  • Public API documentation coverage passes.
  • I verified behavioral coverage, reviewed test necessity and retained acceptance/rejection criteria.
  • Dependencies have been reviewed; no new runtime dependencies.

@yuecideng yuecideng added enhancement Improvements to existing capabilities; use feature for new capabilities. bug Incorrect existing behavior or a fix restoring the intended behavior. task-program Task Program language, semantics, compiler, runtime and integration contracts. task Concrete tasks, deployments, objectives and recipes, including configured Task Program tasks. atomic-skill Atomic Skill goals, planning, execution, verification and recovery. assets Robot and scene assets, meshes, materials, asset catalogs and downloads. motion gen Robot motion planning, MotionGenerator, trajectory generation, retiming and expansion. labels Nov 14, 2026
@yuecideng
yuecideng marked this pull request as ready for review November 14, 2026 15:34
@yuecideng
yuecideng requested a review from han-xudong November 14, 2026 15:34
@yuecideng yuecideng added the devtools Repository development skills, agent context, code checks and contribution tooling. label Nov 14, 2026

@han-xudong han-xudong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed commit 253ae52fb against main. The eight inline findings cover asset materialization, Task Program configuration and inspection, and motion compatibility. The asset materialization regression is merge-blocking and is confirmed by the failed test job.

@han-xudong han-xudong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed b21c0f31 against main. The original eight findings have corresponding fixes. This review records five follow-up findings from checking the fixes and the updated PR diff.

@han-xudong han-xudong left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed 6a07480c. I verified the integration selector, offline Carton preparation, adaptive Place sample floor, and the Pour Water test migration. The four inline findings cover a remaining batch isolation case, two asset-resolution regressions, and the related PickUp sample floor.

for index, target_pose in enumerate(required):
target_pose = build_pose_plan_states(
torch.where(
down_success[:, None, None], target_pose.xpos, current_xpos

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Keep rejected rows out of IK and timing

An FK pose used as a hold target still goes through IK. In a CPU probe with the Franka URDF joint limits and the supported FEP manipulability mode, solving a seed's own TCP pose returned a valid posture with 1.254 rad of joint displacement. At 0.005 rad/s and a 20 ms control period, that rejected row alone sets a lower bound of 23,512 timing intervals, exceeding the 10,000-interval cap that fails the entire batch. Please bypass IK and timing for inactive rows, or supply their joint-space holds directly, for both the approach and retract.

source = Path(task_config_path).resolve()
task = _read(source)
integration = _read(_path(task["task_program"]["integration"], source))
integration = _resolve_integration(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Read asset requirements without resolving runtime manifests

This lookup now resolves DemoSkill and nominal-motion manifests before returning the role contract. I used a deployment with valid asset requirements and a Carton nominal source, then simulated an unavailable Real2SimCarton cache. The role lookup failed on the unrelated runtime asset, whereas the previous lookup could return the same requirements directly. Please share integration selection and override handling here while deferring runtime asset resolution to deployment loading.

)
embodiment_path = _path(raw["embodiment"]["component"], source)
integration = _read(integration_path)
integration = _resolve_integration(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Preserve registry references in materialized deployments

Resolving the integration here turns a DemoSkill {asset, path} reference into a source-machine cache path. The materializer then serializes that absolute path into the generated integration. I materialized a deployment with a real DemoSkill artifact and moved its cache to a different root. Loading the export raised FileNotFoundError without consulting the destination registry. Restoring the original {asset, path} made the same deployment load. Please keep registry references in the exported integration so the destination can resolve them against its own cache.

if options.cartesian_lift:
if motion_policy.strategy != "ik_interp":
raise ValueError("cartesian_lift requires strategy='ik_interp'.")
approach_success, approach_arm = plan_named_arm_trajectory(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P3] Raise the adaptive sample floor for Cartesian PickUp

With adaptive ik_interp, sample_count=9 and the default five gripper frames leave two samples for this approach. The approach needs three keyframes for the start, pre-grasp, and grasp. Using the existing action fixture, the same request planned successfully with the ordinary path and raised ValueError with cartesian_lift=True. Please give adaptive PickUp the same waypoint-based sample floor used by Place.

This branch has not been deployed

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

Labels

assets Robot and scene assets, meshes, materials, asset catalogs and downloads. atomic-skill Atomic Skill goals, planning, execution, verification and recovery. bug Incorrect existing behavior or a fix restoring the intended behavior. devtools Repository development skills, agent context, code checks and contribution tooling. enhancement Improvements to existing capabilities; use feature for new capabilities. motion gen Robot motion planning, MotionGenerator, trajectory generation, retiming and expansion. task Concrete tasks, deployments, objectives and recipes, including configured Task Program tasks. task-program Task Program language, semantics, compiler, runtime and integration contracts.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants