Skip to content

Add delayed implicit actuator support - #8992

Open
AntoineRichard wants to merge 3 commits into
isaac-sim:mainfrom
AntoineRichard:codex/delayed-implicit-actuator
Open

AntoineRichard wants to merge 3 commits into
isaac-sim:mainfrom
AntoineRichard:codex/delayed-implicit-actuator

Conversation

@AntoineRichard

@AntoineRichard AntoineRichard commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Adds DelayedImplicitActuator and DelayedImplicitActuatorCfg to model command latency while retaining solver-side implicit PD control, as requested by Hexagon. Position, velocity, and feed-forward effort targets share one delay per environment, sampled uniformly from [min_delay, max_delay] on reset in physics steps. Joint-state feedback and solver gains remain current.

The implementation shares the existing delayed PD command-history lifecycle, preserves partial-reset history isolation, and documents startup behavior and configuration. Combining delayed implicit groups with native explicit actuator execution in the same articulation raises an actionable error; use use_newton_actuators=False for that combination. Separate articulations can mix both execution paths: Newton retains the environment-owned physics loop whenever a Lab actuator model needs per-step updates, preventing delays from growing with policy decimation.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Validation

Tests reuse existing coverage: the native collection test owns the mixed-group rejection, the existing delayed-PD equivalence test owns explicit-delay compatibility, and the mixed-scene timing regression uses the already-live Newton scene. No additional simulation scene builds are introduced. Independent zero/fixed delay, partial reset, per-environment sampling/channel sharing, and invalid configuration boundaries remain covered.

  • All 26 collection tests passed with kitless selection (pytest ... -m kitless).
  • All 38 Newton actuator integration tests passed, preserving the existing native-only CUDA-graph decimation coverage.
  • Four temporary mutation checks proved the retained tests detect lost explicit delay, skipped physics-step updates, cleared Lab registration, and removal of the mixed-group guard. Every mutation was restored afterward.
  • Full repository formatting, lint, changelog, and Git LFS pointer checks passed; no hooks skipped.
  • Clean Sphinx HTML documentation build passed with warnings treated as errors.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with uv run isaaclab --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have validated the changed behavior with focused coverage and applied the test-audit criteria to test changes
  • I have added a changelog fragment under source/isaaclab/changelog.d/ and source/isaaclab_newton/changelog.d/, with a minor-version marker for the new actuator API
  • My name already exists in CONTRIBUTORS.md

The delayed explicit actuator regression exercises the existing Lab-execution deprecation warning. Existing Newton adapter tests also emit PyTorch export/deprecation warnings; the documentation build was warning-free.

@AntoineRichard
AntoineRichard requested a review from a team November 9, 2026 22:11
@github-actions github-actions Bot added documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Nov 9, 2026

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

Needs attention: Style consistency (3), Test quality (2). Other reviewed categories look good.

  • PR description: The title, scope, and reported validation match the diff; quantify delay-processing overhead with reproducible baseline/candidate revisions, command, workload, hardware, backend, environment count, warmup, step latency, and memory.
  • Design and architecture: Architecture looks good.
  • Scope and over-engineering: No over-engineering issues.
  • API: API changes look good.
  • Compatibility and deprecation: No breaking changes identified.
  • Implementation: Implementation looks good.
  • Style consistency: The new mixin constructor lacks the required return annotation; add -> None to match the contribution guide.
  • Test quality: The added kitless tests lack explicit CI routing markers, so kitless selection skips their delay and reset coverage; mark the qualifying cases.

Minor fixes needed. Posted 5 inline findings.

Automated review; human maintainers own approval decisions.

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

Needs attention: Style consistency (4), Test quality (2). Other reviewed categories look good.

  • PR description: The title and body match the actuator, lifecycle reuse, documentation, and test scope; revision-specific validation is reported, and no measurable performance claim requires benchmark evidence.
  • Design and architecture: Architecture looks good.
  • Scope and over-engineering: No over-engineering issues.
  • API: API changes look good.
  • Compatibility and deprecation: No breaking changes identified.
  • Implementation: Implementation looks good.
  • Style consistency: The extracted constructor lacks the required return annotation; add -> None and remove redundant prose while placing the implicit-model heading above its actual section.
  • Test quality: The native-conflict case marks an implicit-only group native rather than constructing mixed execution; add a disjoint native explicit group and mark the kitless cases for CI selection.

Minor fixes needed. Posted 6 inline findings.

Automated review; human maintainers own approval decisions.

@greptile-apps

greptile-apps Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds DelayedImplicitActuator and DelayedImplicitActuatorCfg to delay commands while keeping solver-side PD control.

  • Implicit actuator groups can delay targets while the solver keeps running PD control.

No actionable issues were found. This review checked source code; it did not run tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Position, velocity, and effort targets] --> B[Shared command history]
  B --> C[Targets delayed per environment]
  C --> D[Solver-side PD control]
  E[Current joint state and gains] --> D
  F[Reset selected environments] --> G[Sample delays and clear selected histories]
  G --> B
Loading

Reviews (1) · Last reviewed commit: "Add delayed implicit actuator support" · Reviewed by Greptile

@AntoineRichard
AntoineRichard requested a review from hujc7 as a code owner November 9, 2026 22:21
@AntoineRichard

Copy link
Copy Markdown
Collaborator Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Nov 10, 2026

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

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant