fix(newton): resolve ArticulationData's per-instance axis for the flat builder

[pascal-roth]

Summary

• _create_simulation_bindings indexed every per-instance sim binding (root pose, joint positions/velocities/limits/targets, body mass/inertia/wrench, tendon properties) with [:, 0], which only holds for the replicated builder's (num_worlds, 1, ...) layout.
• The flat (non-replicated) builder used when replicate_physics=False produces the opposite layout, (1, num_instances, ...), so [:, 0] silently collapsed every non-replicated scene with more than one environment down to a single instance's data for every binding — including write paths used to actually drive the robot (joint position/velocity targets, effort, etc).
• Resolve the per-instance axis dynamically from the root transforms' shape instead of hardcoding it, and reuse that axis for every binding in the method.

Scope note

This fix covers ArticulationData, the asset type exercised by the reproduction below. The same [:, 0] pattern also appears in rigid_object_data.py, cable_object_data.py, and rigid_object_collection.py; I have not touched those since I couldn't verify a fix against a scene that exercises them.

Test plan

• Reproduced against a custom multi-environment task with replicate_physics=False: root_quat_w (and other root/body/joint properties) returned shape (1, ...) instead of (num_envs, ...) before the fix, (num_envs, ...) after.
• CI

[greptile-apps]

Confidence Score: 4/5

The implementation appears safe to merge, with non-blocking follow-up needed for regression coverage and comment accuracy.

Findings

1.  Flat layout lacks coverage ▶
2.  Comment names wrong index ▶

Summary

• Supports replicated (num_worlds, 1, ...) and flat (1, num_instances, ...) layouts.
• Prevents flat scenes from silently exposing or updating only one articulation instance.
• Preserves the existing fixed-base velocity fallback.
• Automated regression coverage for the flat layout is still needed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Read raw root transforms] --> B{Which axis equals num_instances?}
  B -->|Axis 0| C["Replicated index: (:, 0)"]
  B -->|Axis 1| D["Flat index: (0, :)"]
  B -->|Neither| E[Raise RuntimeError]
  C --> F[Bind articulation state and controls]
  D --> F
  F --> G[Per-environment reads and writes]

Loading

[greptile-apps]

Flat layout lacks coverage

This new layout selection controls every articulation state and command binding, but there is no automated regression covering multiple environments with replicate_physics=False. Add a test for the flat (1, num_instances, ...) layout that verifies both a representative read and a command write; otherwise, the original silent instance collapse or a similar indexing regression could return unnoticed.

Knowledge Base Used: Newton backend

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Comment names wrong index

The comment says the code avoids hardcoding [self._instance_index], but self._instance_index is the dynamic index introduced here. The hardcoded expression being replaced was [:, 0]; naming the wrong expression could mislead future changes about what caused the layout bug.

Suggested change

	# Index dynamically instead of hardcoding `[self._instance_index]`, which silently collapsed every
	# Index dynamically instead of hardcoding `[:, 0]`, which silently collapsed every

[isaaclab-review-bot]

Isaac Lab Review Bot

The dynamic per-instance axis resolves the flat-builder layout consistently across articulation bindings. Before merge, add the required isaaclab_newton changelog fragment and correct the misleading indexing comment.

• Design and architecture: Resolving the instance axis once from the root-transform shape and reusing it across bindings is a focused, backend-local solution that preserves the existing replicated layout while supporting the flat builder.
• API: The change restores the documented num_instances-based shapes and write behavior under replicate_physics=False without changing public signatures or dtypes. Because this is user-visible, repository policy requires a changelog fragment for isaaclab_newton.
• Implementation: The binding and buffer paths consistently reuse _instance_index. The new comment incorrectly says the code replaces hardcoded [self._instance_index] indexing, although that is the new form and the replaced expression was [:, 0]; it should be corrected to avoid misleading maintainers.

Minor fixes needed. Posted 2 actionable findings inline.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🟡 Warning · Implementation — Missing isaaclab_newton changelog fragment

This is a user-visible behavior fix (articulation data and write paths now cover all instances for replicate_physics=False), but the change set contains only this source file. Repository rules require one changelog fragment per changed source package for user-visible changes. Add an isaaclab_newton fragment describing the corrected flat-builder instance axis.

[isaaclab-review-bot]

🔵 Suggestion · Implementation — Comment misstates the replaced indexing form

The comment says "instead of hardcoding [self._instance_index]", but that is exactly what the new code does; the replaced form was [:, 0]. This reads as a search-and-replace artifact and conflicts with the nearby inertia comment that still refers to [:, 0], misleading future maintainers about what changed.