Skip to content

Conversation

@AntoineRichard
Copy link
Collaborator

Description

Introduces some tests and fixes for the newton articulation data.

Type of change

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

Sample Benchmarking output: articulation_data_2025-12-11_18-37-03.json

Sample comparison:

===================================================================================================================
BENCHMARK COMPARISON
===================================================================================================================

                                                     BASELINE                        CURRENT
----------------------------------------------------------------------------------------------------
Timestamp:                         2025-12-11T15:56:03.131680     2025-12-11T17:12:07.125690
Commit:                                              d66bc0e3                       d66bc0e3
Branch:                                 antoiner/beta2-hotfix          antoiner/beta2-hotfix

Configuration:                
  Iterations:                                           10000                          10000
  Instances:                                            16384                          16384
  Bodies:                                                  12                             12
  Joints:                                                  11                             11

Hardware:                     
  GPU:                         NVIDIA RTX 5000 Ada Generation Laptop GPU NVIDIA RTX 5000 Ada Generation Laptop GPU

===================================================================================================================
🔴 REGRESSIONS (1 properties)
===================================================================================================================

Property                            Baseline (µs) Current (µs)       Change   % Change   σ Change
-------------------------------------------------------------------------------------------------------------------
joint_acc                                  59.62        74.32       +14.70     +24.7%      +1.5σ

===================================================================================================================
🟢 IMPROVEMENTS (2 properties)
===================================================================================================================

Property                            Baseline (µs) Current (µs)       Change   % Change   σ Change
-------------------------------------------------------------------------------------------------------------------
body_com_quat_b                            86.88        67.81       -19.07     -21.9%      -2.4σ
root_link_vel_b                            72.04        60.59       -11.45     -15.9%      -1.7σ

===================================================================================================================
SUMMARY
===================================================================================================================

  Total properties compared: 71
  🔴 Regressions:     1 (1.4%)
  🟢 Improvements:    2 (2.8%)
  ⚪ Unchanged:      68 (95.8%)

---------------------------------------------

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 11, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 11, 2025

Greptile Overview

Greptile Summary

This PR introduces comprehensive tests, fixes, and benchmarking tools for Newton articulation data. Key changes include:

  • Critical bug fix: Corrected velocity projection formula in velocity_projector kernel where linear and angular velocity components were incorrectly swapped
  • Device parameter additions: Added explicit device parameters to wp.launch() and wp.array() calls throughout articulation_data.py to ensure operations execute on the correct device
  • Improved buffer management: Replaced wp.copy() with .assign() method for cleaner buffer updates
  • State protection: Added is_primed property to prevent modification of default values after initialization
  • Comprehensive testing: New test suite with 3141 lines covering defaults, joint commands, properties, and state transformations using mock interfaces
  • Benchmarking infrastructure: Added benchmarking framework (918 lines) with hardware tracking, git metadata, and statistical comparison tool with sigma-based significance testing

The changes improve correctness, testability, and performance monitoring capabilities of the Newton articulation system.

Confidence Score: 4/5

  • Safe to merge with one minor typo fix recommended
  • Score reflects well-structured changes with comprehensive testing infrastructure. The velocity kernel fix addresses a critical bug. Device parameter additions are systematic and correct. Only issue found is a typo in a docstring ("writting" → "writing"). Extensive test coverage (3141 lines) and benchmarking tools demonstrate thoroughness.
  • All changes look good. The typo in articulation_data.py:383 should be corrected before merge.

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab_newton/isaaclab_newton/kernels/velocity_kernels.py 5/5 Fixed critical bug in velocity_projector function where linear and angular components were swapped, causing incorrect velocity transformations
source/isaaclab_newton/isaaclab_newton/assets/articulation/articulation_data.py 4/5 Added device parameters to wp.launch and wp.array calls, improved buffer management with .assign(), added is_primed property and default value setters with validation
source/isaaclab_newton/test/assets/articulation_data/test_articulation_data.py 4/5 Comprehensive test suite (3141 lines) with parametrized tests covering defaults, joint commands, properties, root/body state, and comparisons with mock data
source/isaaclab_newton/test/assets/articulation_data/benchmark_articulation_data.py 5/5 Benchmarking framework (918 lines) with hardware detection, git tracking, dependency mapping, and JSON export for performance analysis

Sequence Diagram

sequenceDiagram
    participant User
    participant Articulation
    participant ArticulationData
    participant NewtonManager
    participant WarpKernels
    participant TimestampedBuffer

    User->>Articulation: reset()
    Articulation->>ArticulationData: __init__(view, device)
    ArticulationData->>NewtonManager: get_model()
    NewtonManager-->>ArticulationData: model with gravity
    ArticulationData->>ArticulationData: _create_simulation_bindings()
    ArticulationData->>ArticulationData: _create_buffers()
    Note over ArticulationData: Creates TimestampedWarpBuffer<br/>with device parameter
    Articulation->>ArticulationData: set is_primed = True
    Note over ArticulationData: Locks default values<br/>from further modification

    User->>Articulation: update(dt)
    Articulation->>ArticulationData: Access property (e.g., body_com_acc_w)
    
    alt First access or cache invalid
        ArticulationData->>ArticulationData: Check timestamp
        ArticulationData->>WarpKernels: wp.launch(derive_body_acceleration, device=...)
        Note over WarpKernels: Compute acceleration<br/>using velocity_projector
        WarpKernels-->>ArticulationData: Result array
        ArticulationData->>TimestampedBuffer: Update timestamp
        ArticulationData->>ArticulationData: previous_vel.assign(current_vel)
        Note over ArticulationData: Use .assign() instead<br/>of wp.copy()
    else Cache valid
        ArticulationData-->>Articulation: Return cached data
    end
    
    Articulation-->>User: Updated state
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 (1)

  1. source/isaaclab_newton/isaaclab_newton/assets/articulation/articulation_data.py, line 383 (link)

    syntax: "writting" is misspelled

9 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@AntoineRichard AntoineRichard force-pushed the antoiner/beta2_test_articulation_data branch from 7216e34 to ae272ce Compare December 12, 2025 08:30
Copy link
Collaborator

@ooctipus ooctipus left a comment

Choose a reason for hiding this comment

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

LGTM thanks antoine!

@AntoineRichard AntoineRichard force-pushed the antoiner/beta2_test_articulation_data branch from ae272ce to dc7c226 Compare December 15, 2025 11:41
@AntoineRichard AntoineRichard force-pushed the antoiner/beta2_test_articulation_data branch from dc7c226 to 30eee35 Compare December 17, 2025 12:09
@AntoineRichard AntoineRichard force-pushed the antoiner/beta2_test_articulation_data branch from 30eee35 to 4f0cc73 Compare December 17, 2025 16:54
@AntoineRichard AntoineRichard merged commit defdc09 into isaac-sim:dev/newton Dec 17, 2025
4 of 9 checks passed
kellyguo11 pushed a commit that referenced this pull request Dec 17, 2025
# Description

Test & Benchmark for newton articulation. Requires
#4190 to be merged first.

## Type of change

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

## Checklist

- [ ] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants