Revert Built-in Call Optimization Workaround

[christophercrouzet]

Description

This is reverting the workaround introduced in #266.

It's no longer necessary since the following changes in Warp: NVIDIA/warp#801.

Summary by CodeRabbit

• Refactor
  • Streamlined internal transform composition across shapes, joints, and bodies by removing an extra indirection.
  • Simplified transform chaining to use direct operator-based composition, improving clarity of internal logic.
  • Minor performance and maintainability gains in complex simulations.
  • No changes to public APIs or user-facing behavior; existing workflows remain unchanged.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaced a local ctypes-based transform_mul helper with direct transform composition using the multiplication operator across builder internals; updated shape, joint, and body transform call sites. No public API changes.

Changes

Cohort / File(s)	Summary
Transform composition refactor newton/_src/sim/builder.py	Removed local transform_mul and its ctypes dispatch; replaced calls with direct operator-based composition (xform * other_transform) across shape, joint, and body update logic; internal-only change, no external API impact.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Builder
  participant Transform

  rect rgba(190,220,255,0.25)
  note over Builder: Previous flow
  Builder->>Builder: call transform_mul(xform, other)
  Builder->>Transform: ctypes/native dispatch
  Transform-->>Builder: composed_transform
  Builder->>Builder: apply composed_transform
  end

  rect rgba(200,255,200,0.25)
  note over Builder: New flow
  Builder->>Transform: xform * other
  Transform-->>Builder: composed_transform (operator overload)
  Builder->>Builder: apply composed_transform
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title “Revert Built-in Call Optimization Workaround” refers to undoing an optimization workaround but the changes actually remove the transform_mul helper and update builder code to use direct transform multiplication; there is no built-in call optimization reversal in this diff.	Please rename the title to accurately summarize the primary change, for example “Remove transform_mul helper and use direct transform multiplication in builder”.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75105a2 and 7202986.

📒 Files selected for processing (1)

• newton/_src/sim/builder.py (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• newton/_src/sim/builder.py

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
• GitHub Check: Run GPU Benchmarks (Pull Request)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[christophercrouzet]

Depends on #921.

[eric-heiden]

I'm not sure this is ready yet, there is still an up to 2x slowdown shown by the benchmarks for the model initialization.
It also seems to get worse the more environments get instantiated.