Skip to content

Conversation

@GiulioRomualdi
Copy link
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes #4050

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Nov 20, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 20, 2025

Greptile Overview

Greptile Summary

Fixed a critical shape mismatch bug in randomize_joint_parameters that caused runtime errors during environment initialization. The issue occurred when indexing joint parameter tensors - the code was incorrectly applying env_ids[:, None] broadcasting in all cases, creating tensors with shape (num_envs, 1, num_joints) instead of the expected (num_envs, num_joints).

Key Changes:

  • Introduced env_ids_for_slice variable that conditionally applies [:, None] broadcasting only when both env_ids and joint_ids are tensors
  • Applied the fix consistently across all joint parameter writes: friction coefficients (static, dynamic, viscous), armature, and position limits
  • The fix aligns with how downstream methods (e.g., write_joint_friction_coefficient_to_sim) expect to receive parameter tensors

Impact:

  • Resolves RuntimeError: shape mismatch that prevented startup randomization of actuator parameters
  • Maintains correct behavior for all edge cases: when either or both of env_ids/joint_ids are slices vs tensors

Confidence Score: 5/5

  • This PR is safe to merge with high confidence
  • The fix is minimal, well-targeted, and correctly addresses the root cause. The logic properly handles all edge cases (slice vs tensor combinations), follows the same pattern used in downstream methods, and resolves a critical bug that completely blocked the feature
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/envs/mdp/events.py 5/5 Fixed tensor shape mismatch by conditionally applying broadcasting only when both env_ids and joint_ids are tensors

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant RJP as randomize_joint_parameters
    participant Helper as _randomize_prop_by_op
    participant Asset as Articulation Asset
    
    User->>RJP: Call with env_ids & joint_ids (tensors)
    
    Note over RJP: Determine env_ids_for_slice<br/>if both are tensors:<br/>env_ids_for_slice = env_ids[:, None]<br/>else: env_ids_for_slice = env_ids
    
    RJP->>Helper: Call _randomize_prop_by_op(data, env_ids, joint_ids)
    Note over Helper: Returns tensor shape (num_envs, num_joints)
    Helper-->>RJP: friction_coeff tensor
    
    RJP->>RJP: Index: friction_coeff[env_ids_for_slice, joint_ids]
    Note over RJP: Result shape: (num_envs, num_joints)<br/>NOT (num_envs, 1, num_joints)
    
    RJP->>Asset: write_joint_friction_coefficient_to_sim<br/>(static_friction_coeff, env_ids, joint_ids)
    Note over Asset: Asset expects shape:<br/>(len(env_ids), len(joint_ids))
    Asset->>Asset: Internally applies env_ids[:, None] for indexing
    Asset-->>RJP: Success
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

[Bug Report] Runtime error when using randomize_joint_parameters event

1 participant