Skip to content

Fix implicit effort submission in ovphysx - #7782

Open
marcodiiga wants to merge 1 commit into
isaac-sim:developfrom
marcodiiga:fix/malesiani/ovphysx_implicit_effort_submission
Open

Fix implicit effort submission in ovphysx#7782
marcodiiga wants to merge 1 commit into
isaac-sim:developfrom
marcodiiga:fix/malesiani/ovphysx_implicit_effort_submission

Conversation

@marcodiiga

Copy link
Copy Markdown
Collaborator

Description

Implicit actuators on the standard ovphysx path submitted their PD effort estimate as an additional joint force while native joint drives remained enabled. Submit the processed effort command so implicit joints receive only their feedforward effort, while explicit joints retain their computed motor output. This matches the existing PhysX adapter.

Extend the existing tests to cover both actuator execution paths and reordered implicit joints, including force readback, native gains and effort telemetry.

Type of change

  • Bug fix

Validation

  • The original selector fails the ordinary mixed-actuator GPU case and the reordered implicit CPU/GPU cases at the force assertions. The native actuator case already passes.
  • All four focused cases pass with the corrected selector, using separate CPU and GPU processes.
  • Formatting and changelog checks pass.

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Checklist

  • I have read the contribution guidelines.
  • I have run the pre-commit checks with uv run isaaclab -f.
  • I have added regression coverage and a package changelog fragment.
  • My name already appears in CONTRIBUTORS.md.

@marcodiiga
marcodiiga requested a review from a team September 13, 2026 16:31
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Sep 13, 2026
@marcodiiga

Copy link
Copy Markdown
Collaborator Author

run-ci

@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

The change corrects OVPhysX effort submission by sourcing the processed simulation effort target instead of PD-inclusive applied-effort telemetry. Regression coverage checks mixed implicit/explicit actuators, native and standard execution paths, reordered joints, force bindings, gains, and telemetry; the user-visible fix is documented in an isaaclab_ov changelog fragment.

  • Design and architecture: The fix is localized to the OVPhysX backend submission boundary. It preserves native-runtime effort handling and existing user-to-backend reordering while separating implicit actuator feedforward submission from PD-inclusive telemetry.
  • API: No public symbols, signatures, or defaults change. The observable correction is that implicit joints no longer receive their PD estimate twice, while actuators.applied_effort continues to expose PD-inclusive telemetry. The changelog records this behavior change.
  • Implementation: The changed effort source is consistently used by both identity-order and reordered submission paths, with existing capability gating retained. Tests directly verify backend actuation force, native stiffness, explicit actuator output, and applied-effort telemetry across the relevant execution paths. Users relying on the prior double-applied implicit force may observe reduced force, but that behavior was erroneous and is explicitly documented.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

@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 Sep 13, 2026
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with the corrected selector matching existing actuator-buffer and joint-ordering contracts.

Summary

  • Selects processed simulation effort commands for the non-native OVPhysX path.
  • Retains computed motor output for explicit actuators and native-runtime execution.
  • Adds mixed-actuator, nonidentity-ordering, force-readback, gain, and telemetry regression coverage.
  • Documents the fix in a changelog fragment.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Actuator commands] --> B[Actuator collection processing]
    B --> C{Joint type}
    C -->|Implicit| D[Feedforward effort]
    C -->|Explicit| E[Computed and limited motor effort]
    D --> F{Native runtime active?}
    E --> F
    F -->|No| G[_joint_effort_target_sim]
    F -->|Yes| H[Native joint_f_2d]
    G --> I[Optional user-to-backend reorder]
    H --> I
    I --> J[OVPhysX actuation-force binding]
    D --> K[Implicit PD estimate retained as telemetry]
Loading

Reviews (1) · Last reviewed commit: "Fix implicit effort submission in ovphys..."

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

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants