Refactor articulation actuator ownership - #6839
Merged
kellyguo11 merged 151 commits intoAug 19, 2026
Merged
Conversation
github-actions
Bot
added
documentation
AntoineRichard
force-pushed
the
antoiner/actuators-collection-split-6248
branch
from
August 5, 2026 23:35
5c59a09 to
9ce89f5
Compare
AntoineRichard
force-pushed
the
antoiner/actuators-collection-split-6248
branch
4 times, most recently
from
August 7, 2026 14:57
0d670f4 to
d15f91a
Compare
AntoineRichard
force-pushed
the
antoiner/actuators-collection-split-6248
branch
from
August 12, 2026 14:15
4929716 to
6a6049a
Compare
AntoineRichard
marked this pull request as ready for review
August 13, 2026 14:29
AntoineRichard
requested review from
StafaH,
aserifi,
david-cao-mueller,
fatimaanes,
huidongc,
hujc7,
kellyguo11,
marcodiiga,
ooctipus,
pbarejko,
rilei-nvidia and
rubengrandia
as code owners
August 13, 2026 14:29
Contributor
|
Too many files changed for review (176 files, 100 file limit). Bypass the limit by tagging |
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Isaac Lab Review Bot
The backend-neutral ActuatorCollection ownership split is coherent, but two construction-path regressions affect supported custom actuator configurations: direct ActuatorBase subclasses receive unsupported gain keywords, and string class references are validated without being resolved for instantiation.
- Design and architecture: The separation between articulation-owned solver state, collection-owned commands and telemetry, model-owned parameters, and backend-specific control adapters is consistent across PhysX, Newton, and OVPhysX. Before merge, the collection’s group-construction path should preserve the documented ActuatorBase extension contract and consistently use resolved actuator classes.
- API: ActuatorBase subclasses that inherit its constructor now fail because ActuatorCollection unconditionally supplies stiffness and damping even though ActuatorBase.init no longer accepts them. This breaks a documented extension point without a compatibility path. Additionally, string class_type references are treated as accepted inputs during validation but fail later because the unresolved string is inspected and called.
- Implementation: Execution batching, backend submission, alias forwarding, and copied-config resolution are broadly consistent. The actionable defects are localized to ActuatorCollection._build_groups: conditionally pass gain arguments or retain compatibility in ActuatorBase, and resolve class_type once before signature inspection, construction, and grouping.
Minor fixes needed. Posted 2 actionable findings inline.
The full PR diff was reviewed; some supplemental surrounding file context was omitted.
Automated review; human maintainers own approval decisions.
| joint_ids=actuator_joint_ids, | ||
| num_envs=self.num_instances, | ||
| device=self.device, | ||
| stiffness=properties.stiffness, |
Contributor
There was a problem hiding this comment.
🟡 Warning · Api — ActuatorBase subclasses break on gain kwargs
_build_groups always passes stiffness= and damping= into cfg.class_type(**actuator_kwargs), but ActuatorBase.__init__ no longer accepts them (they moved to ImplicitActuator/IdealPDActuator). Actuators deriving directly from ActuatorBase, a documented extension point, now raise TypeError at construction with no deprecation path. Either keep accepting these kwargs in ActuatorBase.__init__, or gate them with the same inspect.signature fallback already used for effort_limit.
| else: | ||
| effort_limit_name = "actuator_effort_limit" | ||
| effort_limit_value = defaults.joint_effort_limit | ||
| constructor_parameters = inspect.signature(actuator_cfg.class_type.__init__).parameters |
Contributor
There was a problem hiding this comment.
🟡 Warning · Implementation — String class_type resolved but not used
_is_implicit_actuator_cfg resolves a string class_type via _resolve_actuator_class, so string references are an accepted config form, but _build_groups then calls inspect.signature(actuator_cfg.class_type.__init__) and actuator_cfg.class_type(**actuator_kwargs) on the unresolved value. A "module:Class" config therefore fails at construction. Resolve the class once and reuse that type for signature inspection, instantiation, and grouping.
Add ActuatorCollection as the backend-neutral owner for actuator state and command APIs. Route legacy articulation target and gain setters through the collection with deprecation warnings. Move common articulation actuator-control forwarding into a shared helper and keep backend adapters focused on command submission, friction writes, and native actuator paths for PhysX, OVPhysX, and Newton. Add changelog fragments and focused ActuatorCollection tests.
Newton-executed groups no longer instantiate Isaac Lab actuator models: the Newton controllers own their parameters, so a Lab model only held misleading construction-time snapshots behind access guards. The collection mapping now returns the owning Newton actuator objects directly, letting users read and modify controller storage without indirection. Newton merges structurally identical joints into one actuator, so several groups can share an object; the collection keeps per-group joint indices and uses them for the group-scoped, user-ordered read_actuator_parameter and write_actuator_parameter access paths. The Newton-managed gain guards on IdealPDActuator are deleted along with the dead construction tensors they protected. The adapter's bind_articulation and build_implicit_dof_mask take implicit joint selectors instead of the Lab actuator mapping, decoupling the Newton binding from Lab model objects entirely. Gain randomization classifies groups by ownership: implicit groups write through the articulation, Newton groups through the parameter door, and Lab explicit groups keep their own tensors.
Three simplifications to the actuator models: - ImplicitActuator setters for articulation-owned properties now warn and ignore the assignment instead of storing pre-binding values or raising. Construction writes go through the private construction buffers, so the store-or-raise state machine served no caller, and a warning pointing at the articulation writers is friendlier than an exception while still refusing the write. - The deprecated ImplicitActuator.effort_limit override is deleted: the base-class alias already forwards to actuator_effort_limit, which implicit models expose as the live articulation joint effort limit. - RemotizedPDActuator no longer resolves deprecated aliases itself or mutates the user's configuration before construction. The base class resolves aliases, and the parsed actuator limits are replaced with infinity afterwards, since the angle-dependent lookup table governs effort clipping for this model.
- Extract resolve_joint_parameter as the single source of joint-parameter resolution semantics; ActuatorBase._parse_joint_parameter and the collection's construction-time property resolution both delegate to it. - Key the implicit executor's cached Warp launch with a monotonic counter instead of id(), which could be reused after garbage collection. - Align documentation with the direct-ownership model: the parameter door documents itself as the group-scoped, user-ordered access path next to raw Newton actuator objects, and _resolve_limit_aliases documents its in-place configuration writes. - Give ImplicitActuator.reset the base-class signature and fold the two gain-default capture loops in randomize_actuator_gains into one.
The concepts page still described the collection as a mapping to Isaac Lab actuator models. Describe the ownership-based mapping: Lab-executed groups return their model instances, Newton-executed groups return the owning Newton actuator objects (shared when Newton merges structurally identical joints), with raw component access next to the group-scoped read_actuator_parameter and write_actuator_parameter paths. Also note that implicit property assignments are ignored with a warning and that plain implicit groups execute through one fused kernel launch.
Cut verified redundancy across the actuator tests without losing behavioral coverage: - Extract the backend-agnostic material shared verbatim by the two Lab-vs-Newton equivalence suites into isaaclab.test.utils.actuator_equivalence: the actuator config catalog, the four equivalence assertion oracles as a mixin, the actuator-state-reset scenario base, the DR mocks, and the neural checkpoint builders. The duplicated Spot knee lookup tables are replaced by imports of the public table in isaaclab_assets. - Delete subsumed equivalence classes: the zero-feedforward implicit twins (the feedforward classes exercise a superset), the mixed explicit class (covered standalone and by mixed-with-implicit), two decimation variants, the physx remotized functional class (implied by its equivalence class), and the newton gain-env-stride class (its absolute assertion moved into the DR test). Drop the private adapter-gather halves of the DR gain assertions in favor of the public parameter reads made alongside them. - Keep the full limit-resolution matrix on the Newton backend only; PhysX and OVPhysX reduce to per-limit smoke tests that verify the resolved values land in each backend's native solver reads. Trim a duplicate actuator_effort_limit parametrize value and constant single-value parametrizes, keeping the ones the sim fixture consumes. - Delete collection tests whose concepts died with explicit aggregation, the legacy-control dynamic-type test, and the runtime alias-property block duplicated by the per-actuator suites; fold the alias forward-when-unset rows into the equivalence matrix. - Remove the dead simulation fixture from the implicit-actuator unit tests (48 pointless sim boots) and collapse the per-limit unit tests in the implicit and ideal-PD suites into parametrized single tests; the two unit files no longer require Isaac Sim at all. Slim the authoring integration test to one actuator group.
The parameter door was the last Newton-only surface on the otherwise backend-neutral ActuatorCollection. With native groups mapping directly to their Newton actuator objects, group-scoped access is explicitly Newton tooling: read_group_parameter and write_group_parameter now live in isaaclab.actuators.newton as free functions taking the collection and group name, next to the selection machinery they are built from. The collection keeps only backend-neutral responsibilities (mapping, commands, telemetry, lifecycle) plus its group joint metadata, which the functions consume for user-ordered columns. Call sites (gain randomization event, deprecated gain-write forwarder, tests) and the documentation move to the new spelling.
ooctipus
reviewed
Aug 18, 2026
| applied_val_log = default_usd_val if cfg_val is None else float(new_val[idx]) | ||
| table.append([name, int(ids[idx]), default_usd_val, cfg_val_log, applied_val_log]) | ||
|
|
||
| def _parse_joint_parameter( |
Collaborator
There was a problem hiding this comment.
this shim looks removable
ooctipus
approved these changes
Aug 18, 2026
ooctipus
left a comment
ooctipus
left a comment
Collaborator
There was a problem hiding this comment.
Thanks this looks much nicer now.
only left a comment about shim layer but otherwise looks like a great rework :D
Congrat
ooctipus
reviewed
Aug 18, 2026
| | gpu_max_rigid_patch_count: 81920 | actuators={ | | ||
| | gpu_found_lost_pairs_capacity: 1024 | "cart_actuator": ImplicitActuatorCfg( | | ||
| | gpu_found_lost_aggregate_pairs_capacity: 262144 | joint_names_expr=["slider_to_cart"], | | ||
| | gpu_total_aggregate_pairs_capacity: 1024 | effort_limit=400.0, | |
Collaborator
There was a problem hiding this comment.
this should be actuator_limit?
ooctipus
reviewed
Aug 18, 2026
| "arm": ImplicitActuatorCfg( | ||
| joint_names_expr=["shoulder_pan_joint", "shoulder_lift_joint", | ||
| "elbow_joint", "wrist_1_joint", "wrist_2_joint", "wrist_3_joint"], | ||
| effort_limit=87.0, # From UR10e specifications |
Collaborator
There was a problem hiding this comment.
would this be actuator_(effort|velocity)_limit?
The command view holds the targets users request and the joint-command view holds what the actuator models produce for the solver; name them for those roles. ActuatorCommand becomes ActuatorTargetCommand exposed as target_command, and ActuatorJointCommand becomes ActuatorOutputCommand exposed as output_command, swept across the backends, data bindings, deprecation messages, tests, tutorials, and documentation.
The superpowers entry excluded a contributor-local worktree stash that does not exist in the repository.
The method had become a one-line wrapper around the module-level resolve_joint_parameter() after the resolution logic moved there. Call sites in the actuator models now call the free function directly, addressing review feedback on the redundant indirection. Custom-actuator backward compatibility was already broken by the constructor rework in this PR, so no deprecation path is kept.
Fix leftover command and joint_command view names in the actuators concept page, and replace the stale 1.0e9 solver effort note: explicit groups now keep the authored joint effort limit, so the solver clips the model output a second time. Document that behavior change with migration guidance in the 3.0 guide and the changelog fragment, point users of the deprecated write_actuator_*_to_sim writers at randomize_actuator_gains and write_group_parameter, and add the isaaclab.actuators.newton group-parameter functions to the API reference so their cross-references resolve.
Export resolve_joint_parameter from isaaclab.actuators so custom actuator subclasses have a public replacement for the removed ActuatorBase._parse_joint_parameter helper. Add the function to the API reference, a migration-guide entry with a before/after snippet for custom actuator authors, and changelog entries for the addition and the removal.
Fix the write_actuator_*_to_sim deprecation warning to point at write_group_parameter instead of the removed ActuatorBase.write_parameter, drop the unasserted private _sim_bind_joint_computed_effort recordings from both actuator equivalence twins, classify implicit groups in randomize_actuator_gains through the is_implicit_model class flag instead of a parallel isinstance tuple, and hoist the local imports in the PhysX benchmark asset runtime.
Forward None instead of slice(None) from Articulation.reset to the actuator collection so delayed-actuator buffers, which reject slices, reset all environments correctly (review feedback). Rename the fake collection in the PhysX prepare test to the target_command view and drop the removed adapter computed-effort field from the shared ordering-trace helper.
The OV articulation was missed when the PhysX and Newton backends were changed to forward None instead of slice(None) to the actuator collection reset. Also migrate the OV gain-event test off the removed group.stiffness/damping accessors to read_group_parameter, fixing the previously failing environment-selective reset and gain test.
Integrates 51 commits from develop. Notable resolutions: - Ported the implicit-actuator rated/solver effort-limit separation (isaac-sim#7078, via the Factory task isaac-sim#6891) into the renamed API: implicit groups honor a configured actuator_effort_limit as a stored rated limit distinct from the joint_effort_limit solver clamp, and the deprecated effort_limit alias now resolves to the rated limit for every actuator type (mirroring to the solver clamp when no separate clamp is configured). Verified the Factory Franka configs resolve with distinct rated and solver limits on both axes. - Kept develop's whole-path prim-regex matching (path_expr_to_glob) in both backend articulations. - Followed develop's test prunes (test_dr_legs_physics_presets.py deleted, reach preset tests trimmed) and kept the consolidated actuator equivalence twins.
Quote the torch union annotation in build_implicit_dof_mask so the module imports under Sphinx's mocked torch, remove the stale ActuatorJointProperties references from the actuators API page, rebuild the four migration-guide grid tables whose columns overflowed during the command-view rename, and wrap an overlong line in the actuator schema authoring.
Import IdealPDActuator lazily in randomize_actuator_gains so the environment-factory import path stays free of actuator config modules (test_factories_are_kitless_in_fresh_process), and point the actuators concept page at the existing policy-transfer how-to instead of a nonexistent document.
hujc7
added a commit
to hujc7/IsaacLab
that referenced
this pull request
Aug 20, 2026
Resolves six conflicts from isaac-sim#6839 (actuator ownership) and isaac-sim#7036 (Shadow Hand prototype spawn): - Carry isaac-sim#7036's spawn_path onto the unified asset so only env_0 is authored. - Adopt joint_effort_limit, the new name for effort_limit_sim. - Keep isaac-sim#6839's actuators.compute/submit_commands delegation and re-apply the MuJoCo tendon actuator write on top. - Restore SHADOW_HAND_NEWTON_CFG as the asset's MuJoCo variant, so the demo and its new test keep working against the unified asset.
5 tasks
This was referenced Aug 27, 2026
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
4 participants
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
This PR adds a backend-neutral actuator runtime around
ActuatorCollectionandseparates actuator-model state from simulated joint state. It keeps one scoped
collection per articulation while presenting one command, processed-command,
telemetry, group-access, and lifecycle API across PhysX, Newton, and OVPhysX.
The two main goals are:
their named configuration or access.
Ownership and public API
Articulationowns actuator application and backend submission.ArticulationDataowns live solver joint properties, including joint gains,solver limits, armature, and friction.
ActuatorCollectionowns articulation-wide commands, processed commands,effort telemetry, group lookup, and actuator lifecycle.
ActuatorControlimplementations own ordering conversion,joint-property writes, command submission, and native-controller integration.
All collection-wide arrays use articulation joint order. Native paths bypass
joint_command, so that view is not submitted-command telemetry for them.Joint properties and actuator properties
actuator_effort_limitis the explicit model clipping limit.joint_effort_limitandjoint_velocity_limitconfigure the joint or solver.The deprecated
effort_limitalias resolves to the actuator limit for explicitmodels and to the joint limit for implicit models.
effort_limit_simandvelocity_limit_simremain deprecated configuration aliases through 3.x.Explicit groups retain their model gains, rated velocity, delay, motor curves,
and clipping limits. They do not retain copies of solver limits or friction.
Implicit groups instead read stiffness, damping, and effort limits from live
articulation buffers because the backend executes their drive.
The former group-level joint-property accessors (
effort_limit_sim,velocity_limit_sim, armature, and friction variants) are removed. Read thosevalues through
ArticulationDataand update them with articulation jointwriters. The legacy backend gain writers remain deprecated 3.x forwarders;
managed randomization should use
randomize_actuator_gains.Logical groups and execution batching
Named groups remain the configuration and access surface. Internally:
one batch even when their parameters differ;
processed commands and telemetry in one fused Warp launch;
compute, cached Warp gathers, and fused output publication;
redundant gather;
separate.
Native runtime parsing aggregates structurally compatible controllers while
keeping per-DOF values. Unsupported custom explicit configurations raise before
USD actuator state is changed. Two ambiguous construction states are rejected:
Backend behavior
native explicit actuators through the shared host adapter.
shared host adapter while preserving eager tensor binding and partial writes.
manager-owned native controllers inside the solver.
only at backend boundaries.
The host adapter captures staging, native model execution, and telemetry on
CUDA when possible and falls back to eager execution otherwise. It manages the
capture for stateful native actuators. Neural checkpoints use Isaac Lab's shared
file cache before Newton metadata is added, so local and remote paths follow the
same loading path.
Compatibility and documentation
actuators.command.ArticulationDatacommand and torque-telemetry accessors forwardto the collection.
Newton-native actuators.
explicit/implicit behavior, batching, native execution, and backend-specific
constraints.
LEAPP action terms retain the annotated articulation setters until the exporter
supports collection setters.
Final performance validation
Both revisions used fresh, isolated environments and the same dependency lock.
Run order was counterbalanced by row and seed. Throughput values below are mean
± sample standard deviation. Throughput changes are paired geometric FPS ratios
with two-sided 95% t intervals; p-values are Holm-adjusted across all 12 rows.
Checkpoint playback: five paired seeds
Protocol: fixed policy checkpoints, 4,096 environments, 50 warm-up steps, and
1,000 measured steps under inference mode. This uses the full benchmark task
configuration rather than the reduced interactive play configuration. The
canonical result set contains 60 paired comparisons and 120 unique bundles.
developFPSThere is no supported playback regression. Six rows remain significant after
correction. The negative Go2 PhysX-native point estimate is small relative to
its seed variance and its interval spans large gains and losses.
Environment creation time (exploratory)
The playback harness also records the time spent in
gym.make(), includingarticulation construction. This is not total process startup: imports, task
configuration, app launch, runner and checkpoint loading, and the first step
are outside this timer. Persistent driver and Warp caches can also affect these
measurements, so the intervals below are unadjusted and should be treated as
follow-up evidence rather than a release gate. A positive change means the PR
was slower to create the environment.
developsecondsMost rows are unresolved. The consistent signal worth a dedicated startup
benchmark is the roughly 0.23-second increase for Go2 on Newton, on both the Lab
and native actuator paths.
Full training: three paired seeds
Protocol: RSL-RL, 4,096 environments, 50 timing warm-up steps, and each task's
full schedule: Cartpole 150, ANYmal-D and Go2 300, Franka 1,000, and G1 1,500
iterations. The canonical set contains 36 paired comparisons and 72 unique
successful bundles. Native neural runs are excluded because that path is known
not to work on either revision.
developFPSNo row has a negative mean training-throughput change. Cartpole on both
backends and G1 on Newton remain significant after correction.
Each run's late-training reward is averaged over its final 100 iterations. The
table reports the mean ± sample standard deviation across three seeds:
developrewardEpisode length and success were also checked over the final 100 iterations. No
convergence regression is statistically established.
G1 PhysX has a lower mean reward on the PR, but the paired difference is
unresolved (
p = 0.157) and success is unchanged. ANYmal-D on Newton isbimodal across seeds; its apparent improvement is also unresolved (
p = 0.182).Compared commits:
develop:135cf98904cbd73f3b8bdd89188e1a463ebcbb979ee6296124687ec54ed13e102ac59b7667b08d8dValidation
Fused implicit telemetry can differ from the previous Torch expression by one
float32 ULP (
4.768e-7) because of arithmetic ordering. Processed position,velocity, and feed-forward effort commands sent to the backend remain exact.
Type of change
Checklist
./isaaclab.sh -fCONTRIBUTORS.md