feat(gen-sim): add TaskSpec v0.1 and guarded E2 acceptance - #613
Open
yuecideng wants to merge 5 commits into
Open
feat(gen-sim): add TaskSpec v0.1 and guarded E2 acceptance#613yuecideng wants to merge 5 commits into
yuecideng wants to merge 5 commits into
Conversation
yuecideng
added
enhancement
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1303
to
+1305
| if guard_only and context.robot.timestamp >= request.deadline: | ||
| failure_mask = request.env_mask.clone() | ||
| observed_mask.zero_() |
There was a problem hiding this comment.
In a vector rollout, one row can satisfy its held-object guard while another remains unresolved. Because the command cursor is shared, both rows remain in request.env_mask; when the deadline is reached, this branch marks the entire mask as failed. It therefore invalidates the already-verified row as well as the unresolved row, breaking row-local execution outcomes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/lab/task_program/runtime/executor.py
Line: 1303-1305
Comment:
**Guard timeout fails peer rows**
In a vector rollout, one row can satisfy its held-object guard while another remains unresolved. Because the command cursor is shared, both rows remain in `request.env_mask`; when the deadline is reached, this branch marks the entire mask as failed. It therefore invalidates the already-verified row as well as the unresolved row, breaking row-local execution outcomes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+195
to
+203
| try: | ||
| final = observe(env, binding, scope="task_goal", output=output) | ||
| except Exception as exc: | ||
| final = { | ||
| "status": ["unavailable"] * num_envs, | ||
| "error": {"type": type(exc).__name__, "message": str(exc)}, | ||
| } | ||
| write_evidence( | ||
| output / "task_evaluation.json", |
There was a problem hiding this comment.
If final acceptance has already written task_evaluation.json and a later metadata hook or dataset-saving reset raises, the outer exception handler calls this function. It re-observes the environment and overwrites both the goal-state artifact and the evaluation report. This loses the original program and task result and can replace the pre-reset observation with partially reset state, so the retained evidence no longer represents the acceptance boundary it was meant to freeze.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/gen_sim/task_engine/_task_spec.py
Line: 195-203
Comment:
**Failure overwrites frozen evidence**
If final acceptance has already written `task_evaluation.json` and a later metadata hook or dataset-saving reset raises, the outer exception handler calls this function. It re-observes the environment and overwrites both the goal-state artifact and the evaluation report. This loses the original program and task result and can replace the pre-reset observation with partially reset state, so the retained evidence no longer represents the acceptance boundary it was meant to freeze.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+36
to
+39
| if pose.ndim != 3 or pose.shape[-2:] != (4, 4) or local_axis.shape != (3,): | ||
| raise ValueError("Expected poses (N, 4, 4) and one local axis (3,).") | ||
| axis = pose[:, :3, :3].double() @ local_axis.double() | ||
| return torch.atan2(torch.linalg.vector_norm(axis[:, :2], dim=-1), axis[:, 2]) |
There was a problem hiding this comment.
This public API documents local_axis as a unit vector but validates only its shape. A caller can pass a zero axis, for which atan2(0, 0) produces a plausible zero tilt that downstream threshold checks may treat as upright. Validating or normalizing finite, nonzero axes here would prevent misleading orientation measurements.
Prompt To Fix With AI
This is a comment left during a code review.
Path: embodichain/compute/task_predicates.py
Line: 36-39
Comment:
**Invalid axes yield zero tilt**
This public API documents `local_axis` as a unit vector but validates only its shape. A caller can pass a zero axis, for which `atan2(0, 0)` produces a plausible zero tilt that downstream threshold checks may treat as upright. Validating or normalizing finite, nonzero axes here would prevent misleading orientation measurements.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
First implementation increment of the TaskSpec/GenSim consolidation design,
targeting
ljd/action_engine_refactor(#531), not main. Baseline verified:c414b8d158615bbe52e448bf67f96206ae7e7a2b. Builds on #531; no issue auto-close.Delivered
embodichain.task_specv0.1: five strict records,bounded predicate AST, canonical role IDs, exact SI decimal normalization,
restricted semantic equivalence/hashing, content references and fail-closed
certificate record validation. Legacy candidate step hashes keep their meaning.
remains gated until measured instance/witness qualification exists.
binds existing acquire/release gates and held guards; GenSim does not create
another executor. New held-move protection is versioned opt-in so legacy
lowerer configurations remain usable.
--task-templateE2 observed-goal acceptance. A strict sidecar andfingerprint/v3 link template to graph/v1; old no-TaskSpec bundles remain v2.
Upright segment thresholds derive from the template. Only single-object
local-+Z upright templates/assets are admitted; process/unsupported predicates
fail early. CLI defaults to supported dual_franka and rejects unsupported
profiles before generation.
reject-only Gym final-acceptance hook before metadata finalization/save/reset.
Initial invalid/unavailable/trivial batches abort without saving. Final rows
are evaluated independently. Exceptional rollouts freeze failed/unavailable
final evidence before abort reset. Reused evidence directories are rejected.
runtime-probe checks do not masquerade as successful static feasibility;
IK, collision-path and physical checks explicitly remain not_run.
Deliberate qualification boundary / follow-up
This is not completion of all P0–P5 and not a physical success certificate.
The E2 report says
qualification=observed_goal_onlyandcertificate_status=unavailable. Full asset-content closure, measuredSceneInstance identity, successful ActionWitness/certificate assembly,
TaskAgent seed migration/template-driven scene requests, other task-family
evaluators, process monitoring, decoder consolidation, Gradio migration and
motion GenerationSession hosting remain dependent increments. No simulator
step/reset/retry/commit owner was added. See
embodichain/task_spec/README.mdand
IMPLEMENTATION.mdfor input limits and remaining exits.Validation
assertion deselected. Coverage includes identity/strict records, semantic
compilation, runtime/atomic failure injection, old/new bundle composition,
actual host metadata/reset ordering, feasibility/workflow, context and docs.
black .,git diff --check, context check/routing and APIcoverage passed (2202/2202 exports).
matched the new TaskSpec/evaluation pages or modules.
failed-rollout/stale evidence, unresolved guard-only motion and misspelled
registered protected phases, with regression tests.
CPU commands use Python 3.11,
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1,-q -c /dev/null --noconftest -p no:cacheprovider, and explicitPYTHONPATH=<worktree>:/root/sources/dexsim/build_Release/lib/python_package.The compiled DexSim package is used for imports only; no simulator is launched.
Known unrelated checks: packaged workflow test expects 8000 steps while this
baseline already configures 10000; that assertion is excluded from the final
focused run. A broader configured open_drawer test attempted an external
Drawer.zip download and failed checksum verification (81 other tests passed);
the existing asset loader removed its invalid cache. No GPU rollout,
multi-seed robustness or remote generation service was run.
Type of change
Screenshots
Not applicable: protocol, compiler/runtime contracts and CPU evidence paths.
Checklist
black .before commits.