Skip to content

Conversation

@kellyguo11
Copy link
Contributor

Description

Updates dependencies to support ARM systems and Spark.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

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 the isaac-lab Related to Isaac Lab team label Dec 18, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 18, 2025

Greptile Summary

This PR refactors dependency management to support ARM64 (aarch64/arm64) architectures alongside existing x86_64 support by migrating from runtime platform checks to PEP 508 environment markers.

Key Changes:

  • Replaced runtime platform.system() checks with declarative PEP 508 markers for platform-specific dependencies
  • Added ARM64-compatible alternatives: daqp==0.7.2, usd-exchange>=2.1.0a3
  • Added onnxscript>=0.5 to rsl-rl extras for Linux aarch64 compatibility
  • Restricted dex-retargeting==0.5.0 and usd-core==25.05.0 to x86_64/AMD64 only

Critical Issue:
The PEP 508 marker syntax uses incorrect formatting - platform_machine in 'x86_64,AMD64,aarch64,arm64' should use a tuple of strings: platform_machine in ('x86_64', 'AMD64', 'aarch64', 'arm64'). This syntax error will prevent pip from correctly parsing the environment markers, causing installation failures.

Confidence Score: 2/5

  • This PR has a critical syntax error in PEP 508 markers that will cause installation failures
  • The PR introduces a critical syntax error in the PEP 508 environment markers across two files (isaaclab/setup.py and isaaclab_newton/setup.py). The incorrect format 'platform_machine in "x86_64,AMD64,aarch64,arm64"' will cause pip to fail parsing the markers, preventing successful installation on any platform. While the approach of using PEP 508 markers is correct and the dependency choices appear reasonable, the syntax error makes this unsafe to merge without correction.
  • source/isaaclab/setup.py and source/isaaclab_newton/setup.py require immediate attention to fix PEP 508 marker syntax

Important Files Changed

Filename Overview
source/isaaclab/setup.py Updated dependency handling to use PEP 508 markers for platform-specific packages (pin-pink, daqp, dex-retargeting), removing runtime platform check
source/isaaclab_newton/setup.py Moved usd-core to conditional install with platform markers, added usd-exchange for ARM64 support
source/isaaclab_rl/setup.py Added onnxscript>=0.5 dependency to rsl-rl extras for Linux aarch64 compatibility

Sequence Diagram

sequenceDiagram
    participant User
    participant pip
    participant setup.py
    participant Platform

    User->>pip: pip install isaaclab/isaaclab_newton/isaaclab_rl
    pip->>setup.py: Read setup.py
    setup.py->>setup.py: Define INSTALL_REQUIRES list
    setup.py->>setup.py: Append platform-specific deps with PEP 508 markers
    Note over setup.py: SUPPORTED_ARCHS_ARM = "platform_machine in ('x86_64','AMD64','aarch64','arm64')"<br/>SUPPORTED_ARCHS = "platform_machine in ('x86_64','AMD64')"
    pip->>Platform: Evaluate platform_system and platform_machine
    Platform-->>pip: Return 'Linux' and 'aarch64' (ARM) or 'x86_64'
    
    alt ARM64 Architecture (aarch64/arm64)
        pip->>pip: Install pin-pink==3.1.0 ✓
        pip->>pip: Install daqp==0.7.2 ✓
        pip->>pip: Install usd-exchange>=2.1.0a3 ✓
        pip->>pip: Skip dex-retargeting (x86_64 only)
        pip->>pip: Skip usd-core (x86_64 only)
        pip->>pip: Install onnxscript>=0.5 (with rsl-rl extra)
    else x86_64/AMD64 Architecture
        pip->>pip: Install pin-pink==3.1.0 ✓
        pip->>pip: Install daqp==0.7.2 ✓
        pip->>pip: Install dex-retargeting==0.5.0 ✓
        pip->>pip: Install usd-core==25.05.0 ✓
        pip->>pip: Install usd-exchange>=2.1.0a3 ✓
        pip->>pip: Install onnxscript>=0.5 (with rsl-rl extra)
    end
    
    pip-->>User: Installation complete
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.

Additional Comments (3)

  1. source/isaaclab/setup.py, line 57-58 (link)

    syntax: PEP 508 marker syntax issue: the in operator requires a tuple of quoted strings, not a single string with comma-separated values

  2. source/isaaclab_newton/setup.py, line 44-45 (link)

    syntax: PEP 508 marker syntax issue: the in operator requires a tuple of quoted strings, not a single string with comma-separated values

  3. source/isaaclab_newton/setup.py, line 47 (link)

    style: Incorrect comment - this dependency is not related to pink_ik, it should reference USD functionality

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@kellyguo11 kellyguo11 merged commit 7ec1197 into isaac-sim:dev/newton Dec 19, 2025
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants