Remove deprecated joint target aliases and deprecate the DOF target layout (#3617)

Co-authored-by: Eric Heiden <eheiden@nvidia.com>

Author: adenzler-nvidia

CHANGELOG.md

@@ -96,9 +96,11 @@
 - Deprecate `SensorTiledCamera(..., config=...)` in favor of `SensorTiledCamera(..., default_render_config=...)`; migrate constructor calls that pass a render config to the new keyword.
 - Deprecate `SensorTiledCamera.render_config` in favor of `SensorTiledCamera.default_render_config`; migrate `sensor.render_config.enable_shadows = True` to `sensor.default_render_config.enable_shadows = True`.
 - Deprecate `SensorTiledCamera.utils.compute_pinhole_camera_rays()` in favor of `SensorTiledCamera.utils.compute_camera_rays_pinhole()`.
+- Deprecate the legacy DOF-shaped `joint_target_q` layout (`newton.use_coord_layout_targets = False`) for models whose joint coordinate and DOF counts differ (free/ball/distance joints); `ModelBuilder.finalize()` now emits a `DeprecationWarning` for such models. Set `newton.use_coord_layout_targets = True` before building models and index targets via `Model.joint_target_q_start`. A future release will make the coordinate layout the only layout and remove the flag.
 
 ### Removed
 
+- Remove the deprecated `joint_target_pos` / `joint_target_vel` aliases from `Model`, `Control`, and `ModelBuilder` (deprecated in 1.3.0); use `joint_target_q` / `joint_target_qd` instead. Reading or assigning the removed names raises `AttributeError` naming the replacement, so a stale `control.joint_target_pos = targets` fails loudly instead of being silently ignored. `Actuator` now always defaults `control_target_pos_attr` / `control_target_vel_attr` to the canonical `joint_target_q` / `joint_target_qd` names; passing `None` explicitly selects the same defaults.
 - Remove the deprecated SDF compatibility attributes `Model.shape_sdf_index`, `Model.texture_sdf_data`, `Model.texture_sdf_coarse_textures`, `Model.texture_sdf_subgrid_textures`, `Model.texture_sdf_subgrid_start_slots`, `Model.sdf_block_coords`, `Model.sdf_index2blocks`, and `SDF.texture_block_coords` (deprecated in 1.3.0); the hydroelastic broadphase derives block coordinates arithmetically and the remaining storage is internal.
 - Remove the deprecated `newton.geometry.build_bvh_shape()`, `refit_bvh_shape()`, `build_bvh_particle()`, and `refit_bvh_particle()` helpers (deprecated in 1.3.0); use `Model.bvh_build_shapes()`, `Model.bvh_refit_shapes()`, `Model.bvh_build_particles()`, and `Model.bvh_refit_particles()` instead.
 - Remove the deprecated `Model.has_heightfields` property (deprecated in 1.3.0); use `Model.heightfield_count`, or `model.heightfield_count > 0` for boolean checks, instead.
@@ -155,6 +157,7 @@
 - Fix `FastKitchenG1` ASV metrics to build the kitchen scene instead of a plain G1 model.
 - Fix the `diffsim_bear` example crashing with its default CUDA configuration and diverging after a few training iterations.
 - Fix masked PID state reset to execute on the integral-state device. (#3447)
+- Fix `eval_inverse_dynamics_passive()` reading past a DOF-sized scratch buffer under `newton.use_coord_layout_targets = True`, producing intermittent NaNs for models with free, ball, or distance joints.
 - Fix MJCF imports ignoring `fromto` transforms and lengths on sites.
 - Reject invalid hollow primitive shell thickness before computing inertia.
 - Fix `ModelBuilder.add_mjcf()` ignoring positive explicit mass on mesh geoms. (#3595)

docs/migration.rst

@@ -146,14 +146,14 @@ corresponds to :attr:`newton.JointTargetMode.EFFORT` together with
 Joint-target layout (``newton.use_coord_layout_targets``)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Historically ``Control.joint_target_pos`` was shaped ``(joint_dof_count,)`` — the same layout as
-:attr:`~newton.State.joint_qd` — even though position targets semantically match
-:attr:`~newton.State.joint_q`. The two layouts diverge whenever an articulation contains a free
-joint (7 coords vs. 6 DOFs) or ball joint (4 coords vs. 3 DOFs); every actuated DOF downstream
-then ends up indexed with the wrong stride.
+In ``warp.sim`` the target array was DOF-shaped like ``joint_qd``. Newton is moving position
+targets to the coordinate layout of :attr:`~newton.State.joint_q` instead, since that is what a
+position semantically is: the two layouts diverge whenever an articulation contains a free or
+distance joint (7 coords vs. 6 DOFs) or ball joint (4 coords vs. 3 DOFs), and under the DOF layout
+every actuated DOF downstream of such a joint is indexed with the wrong stride.
 
-Newton 1.3 introduces an opt-in flag to switch ``Model.joint_target_q`` / ``Control.joint_target_q``
-to the coord-aligned layout that matches ``joint_q``:
+New code should opt into the coordinate layout, which will become the only layout in a future
+release:
 
 .. code-block:: python
 
@@ -167,26 +167,25 @@ to the coord-aligned layout that matches ``joint_q``:
    # model.joint_target_q  has shape (joint_coord_count,)  — matches joint_q
    # model.joint_target_qd has shape (joint_dof_count,)    — matches joint_qd
 
-Migration steps:
-
-- Replace ``Control.joint_target_pos`` / ``Model.joint_target_pos`` with
-  :attr:`Control.joint_target_q` / :attr:`Model.joint_target_q`. The legacy names emit a
-  :class:`DeprecationWarning` and raise :class:`AttributeError` when
-  ``newton.use_coord_layout_targets`` is ``True``.
-- Replace ``Control.joint_target_vel`` / ``Model.joint_target_vel`` with
-  :attr:`Control.joint_target_qd` / :attr:`Model.joint_target_qd`.
-- On :class:`ModelBuilder`, ``joint_target_pos`` and ``joint_target_vel`` have been removed.
-  Configure per-axis targets via :attr:`ModelBuilder.JointDofConfig.target_pos` /
-  :attr:`~ModelBuilder.JointDofConfig.target_vel` before calling ``add_joint*()``, or write
-  to :attr:`ModelBuilder.joint_target_q` / :attr:`~ModelBuilder.joint_target_qd` directly.
+Under the coordinate layout, ball and free joint position targets are quaternions (plus a
+translation for free joints), matching ``joint_q``. Migration notes coming from ``warp.sim``:
+
+- Replace writes to the ``warp.sim`` target arrays with
+  :attr:`Control.joint_target_q` / :attr:`Control.joint_target_qd`. Configure per-axis initial
+  targets via :attr:`ModelBuilder.JointDofConfig.target_pos` /
+  :attr:`~ModelBuilder.JointDofConfig.target_vel` before calling ``add_joint*()``, or write to
+  :attr:`ModelBuilder.joint_target_q` / :attr:`~ModelBuilder.joint_target_qd` directly.
 - When indexing ``joint_target_q`` from user code, use :attr:`Model.joint_target_q_start` (which
   aliases :attr:`Model.joint_q_start` when the flag is ``True`` and
   :attr:`Model.joint_qd_start` otherwise). Solvers and the actuator library already do this.
 - When constructing an :class:`Actuator` with a custom ``pos_indices``, drop the
   ``target_pos_indices`` argument: with the coord layout it defaults to ``pos_indices``.
 
-A subsequent release will flip the flag's default to ``True`` and remove the legacy
-attributes.
+The default is still the legacy DOF-shaped layout for backward compatibility with existing Newton
+code, but it is deprecated: building a model whose joint coordinate and DOF counts differ
+(free/ball/distance joints) under ``use_coord_layout_targets = False`` emits a
+:class:`DeprecationWarning` from ``finalize()``. For models without such joints the two layouts
+are identical, so no warning is emitted and the switch is invisible.
 
 
 ``ModelBuilder``

docs/solvers/mujoco.rst

@@ -434,7 +434,7 @@ angular axis behaves as a revolute closure and three as a ball closure;
 any other configuration is skipped.
 
 Only the kinematic coupling implied by the joint type is enforced. Any
-drive (``joint_target_pos`` / ``joint_target_vel``, PD gains,
+drive (``joint_target_q`` / ``joint_target_qd``, PD gains,
 ``control.joint_f``), joint limits, armature, friction, and
 effort/velocity limits authored on the loop-closing joint are **ignored**
 by :class:`~newton.solvers.SolverMuJoCo`. Loop-joint DOFs and coordinates

newton/__init__.py

@@ -14,26 +14,28 @@
 use_coord_layout_targets: bool = False
 """Use :attr:`joint_q`-aligned layout for joint position targets.
 
-When ``False`` (the default in 1.3), :class:`~newton.Model` and
-:class:`~newton.Control` expose :attr:`joint_target_pos` and
-:attr:`joint_target_vel`, both shaped ``(joint_dof_count,)``. Accessing these
-attributes emits a :class:`DeprecationWarning` since the position-target layout
-is misaligned with :attr:`~newton.State.joint_q` whenever an articulation
-contains a free or ball joint upstream of a position-controlled DOF.
+Controls the shape of :attr:`~newton.Model.joint_target_q` and
+:attr:`~newton.Control.joint_target_q`:
 
-When ``True``, :class:`~newton.Model` and :class:`~newton.Control` instead
-expose:
-
-- :attr:`joint_target_q` with shape ``(joint_coord_count,)``, matching
+- ``True``: shape ``(joint_coord_count,)``, matching
   :attr:`~newton.State.joint_q`.
-- :attr:`joint_target_qd` with shape ``(joint_dof_count,)``, matching
-  :attr:`~newton.State.joint_qd` (same layout as the legacy
-  :attr:`joint_target_vel`).
+- ``False`` (the default): legacy shape ``(joint_dof_count,)``, which is
+  misaligned with :attr:`~newton.State.joint_q` whenever an articulation
+  contains a free, ball, or distance joint upstream of a position-controlled
+  DOF.
+
+:attr:`joint_target_qd` is shaped ``(joint_dof_count,)`` in both layouts,
+matching :attr:`~newton.State.joint_qd`.
+
+Solvers, the actuator library, importers, and viewers honor this flag. Toggle
+it before constructing a :class:`~newton.ModelBuilder`.
 
-Solvers, the actuator library, importers, and viewers honor this flag and read
-whichever attributes are active. Toggle the flag before constructing a
-:class:`~newton.ModelBuilder`; a subsequent release will flip the default to
-``True``, then remove the flag and the legacy attributes.
+.. deprecated:: 1.5
+    The legacy DOF-shaped layout is deprecated. In a future release the
+    coordinate layout becomes the only layout and this flag is removed;
+    ``finalize()`` warns when building an affected model (one whose joint
+    coordinate and DOF counts differ) under ``False``. Set the flag to
+    ``True`` now to migrate.
 """
 
 __all__ = [

newton/_src/actuators/actuator.py

@@ -88,8 +88,8 @@ def __init__(
         effort_indices: wp.array[wp.uint32] | None = None,
         state_pos_attr: str = "joint_q",
         state_vel_attr: str = "joint_qd",
-        control_target_pos_attr: str | None = None,
-        control_target_vel_attr: str | None = None,
+        control_target_pos_attr: str | None = "joint_target_q",
+        control_target_vel_attr: str | None = "joint_target_qd",
         control_feedforward_attr: str | None = "joint_act",
         control_output_attr: str = "joint_f",
         control_computed_output_attr: str | None = None,
@@ -107,8 +107,8 @@ def __init__(
                 ``state.joint_q``). Defaults to *indices*. Differs from
                 *indices* when position and velocity arrays have different
                 layouts (e.g. floating-base or ball-joint articulations).
-            target_pos_indices: Indices into ``control.joint_target_pos`` /
-                ``joint_target_q``. Defaults to *pos_indices* when
+            target_pos_indices: Indices into ``control.joint_target_q``.
+                Defaults to *pos_indices* when
                 :attr:`newton.use_coord_layout_targets` is ``True`` (coord
                 layout), otherwise to *indices* (legacy DOF layout). The flag is
                 read once here, so toggling ``newton.use_coord_layout_targets``
@@ -119,13 +119,9 @@ def __init__(
             state_pos_attr: Attribute on sim_state for positions.
             state_vel_attr: Attribute on sim_state for velocities.
             control_target_pos_attr: Attribute on sim_control for target positions.
-                ``None`` (default) resolves at construction time based on
-                :data:`newton.use_coord_layout_targets`: ``True`` →
-                ``"joint_target_q"``; ``False`` → legacy ``"joint_target_pos"``.
+                ``None`` selects the default ``"joint_target_q"``.
             control_target_vel_attr: Attribute on sim_control for target velocities.
-                ``None`` (default) resolves at construction time based on
-                :data:`newton.use_coord_layout_targets`: ``True`` →
-                ``"joint_target_qd"``; ``False`` → legacy ``"joint_target_vel"``.
+                ``None`` selects the default ``"joint_target_qd"``.
             control_feedforward_attr: Attribute on sim_control for feedforward effort. None to skip.
             control_output_attr: Attribute on sim_control for clamped output effort.
             control_computed_output_attr: Attribute on sim_control for raw (pre-clamp)
@@ -159,31 +155,11 @@ def __init__(
 
         self.state_pos_attr = state_pos_attr
         self.state_vel_attr = state_vel_attr
-        if control_target_pos_attr is None or control_target_vel_attr is None:
-            import warnings  # noqa: PLC0415
-
-            import newton  # noqa: PLC0415
-
-            if newton.use_coord_layout_targets:
-                default_pos_attr, default_vel_attr = "joint_target_q", "joint_target_qd"
-            else:
-                default_pos_attr, default_vel_attr = "joint_target_pos", "joint_target_vel"
-                warnings.warn(
-                    "Actuator default control_target_pos_attr/control_target_vel_attr "
-                    "currently resolves to legacy 'joint_target_pos'/'joint_target_vel' "
-                    "under newton.use_coord_layout_targets=False. The default will switch "
-                    "to canonical 'joint_target_q'/'joint_target_qd' in a future release. "
-                    "Pass control_target_pos_attr='joint_target_q' (and the velocity "
-                    "counterpart) explicitly to lock in the new behaviour now.",
-                    DeprecationWarning,
-                    stacklevel=2,
-                )
-        self.control_target_pos_attr = (
-            control_target_pos_attr if control_target_pos_attr is not None else default_pos_attr
-        )
-        self.control_target_vel_attr = (
-            control_target_vel_attr if control_target_vel_attr is not None else default_vel_attr
-        )
+        # These used to default to None and resolve against the target layout.
+        # Normalize so callers still passing None explicitly keep working
+        # instead of tripping getattr() with a non-string name in step().
+        self.control_target_pos_attr = "joint_target_q" if control_target_pos_attr is None else control_target_pos_attr
+        self.control_target_vel_attr = "joint_target_qd" if control_target_vel_attr is None else control_target_vel_attr
         self.control_feedforward_attr = control_feedforward_attr
         self.control_output_attr = control_output_attr
         self.control_computed_output_attr = control_computed_output_attr