SolverMuJoCo: Support kinematic joints via high-armature DOFs#1667
Open
vreutskyy wants to merge 20 commits intonewton-physics:mainfrom
Open
SolverMuJoCo: Support kinematic joints via high-armature DOFs#1667vreutskyy wants to merge 20 commits intonewton-physics:mainfrom
vreutskyy wants to merge 20 commits intonewton-physics:mainfrom
Conversation
Add KinematicMode enum (NONE, VELOCITY, POSITION) as a per-joint property that emulates kinematic behavior via high armature (1e10) on joint DOFs. The MuJoCo solver sees proper velocity for contact resolution instead of position-teleported mocap bodies. - Add KinematicMode enum to joints.py - Add kinematic parameter to add_joint(), add_joint_free(), add_joint_revolute(), add_joint_prismatic(), add_joint_ball(), and add_body() - Add control.kinematic_target array for setting targets - Add apply_kinematic_target_kernel that writes velocity to joint_qd before each MuJoCo step - Add example with rotating kinematic platform and capsule creatures with position-controlled kinematic hinges - Add 9 unit tests including multi-world support Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-joint kinematic support: new KinematicType enum, per-joint kinematic fields in ModelBuilder/Model, Control.kinematic_target, a Warp kernel to apply kinematic targets, SolverMuJoCo integration to run that kernel pre-step, docs, an example, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Builder as ModelBuilder
participant Model
participant Control
participant Solver as SolverMuJoCo
participant Kernel as apply_kinematic_target_kernel
participant State as JointState
User->>Builder: add_joint(..., kinematic=KinematicType.VELOCITY/POSITION)
Builder->>Builder: store joint_kinematic_type, override armature if kinematic
Builder->>Model: finalize() -> export joint_kinematic_type
User->>Control: set control.kinematic_target (wp.array)
User->>Solver: step(model, control, state)
Solver->>Kernel: launch(joint_kinematic_type, joint_type, joint_q, kinematic_target, inv_dt, ...)
Kernel->>State: read joint_q, kinematic_target, joint metadata
Kernel->>State: compute joint_qd (VELOCITY copy or POSITION→velocity/quaternion delta)
Kernel->>Solver: write joint_qd into state_in
Solver->>Solver: continue physics step with modified joint_qd
Solver->>User: return updated simulation state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/sim/builder.py (1)
2709-2720:⚠️ Potential issue | 🟡 MinorDocument the new
kinematicparameter in public APIs.The new parameter isn’t reflected in the Args sections for these public methods, so the generated API docs will be incomplete. Please add concise descriptions (meaning of
KinematicModeand default) and clarify how it differs from “massless body” notes where applicable, then regenerate the API docs.As per coding guidelines, "Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values" and "Keep auto-generated documentation up-to-date by running docs/generate_api.py when adding new public API files or symbols".
Also applies to: 2780-2794, 2960-2985, 3055-3080, 3148-3162, 3269-3280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 2709 - 2720, The public method add_body needs its docstring updated to document the new kinematic: KinematicMode parameter—add a Google-style Args entry describing KinematicMode (enumeration meaning), the default (KinematicMode.NONE), and a concise note how kinematic differs from a massless body (kinematic controls motion semantics vs massless affecting dynamics calculations); apply the same docstring addition to all other public methods in this module that accept kinematic, ensure return value and behavior notes remain accurate, and then run docs/generate_api.py to regenerate the API docs.
🧹 Nitpick comments (5)
newton/_src/solvers/mujoco/kernels.py (2)
1975-2018: Duplicate quaternion-to-angular-velocity logic between FREE and BALL branches.Lines 1975–1993 (FREE angular part) and Lines 2004–2018 (BALL) are identical quaternion-difference-to-omega computations. Consider extracting a
@wp.functo reduce duplication.♻️ Proposed refactor
+@wp.func +def _quat_diff_to_angular_vel(cur_q: wp.quat, tgt_q: wp.quat, inv_dt: float) -> wp.vec3: + """Compute angular velocity from quaternion difference: omega = axis_angle(tgt * inv(cur)) / dt.""" + dq = tgt_q * wp.quat_inverse(cur_q) + if dq[3] < 0.0: + dq = wp.quat(-dq[0], -dq[1], -dq[2], -dq[3]) + sin_half = wp.sqrt(dq[0] * dq[0] + dq[1] * dq[1] + dq[2] * dq[2]) + omega = wp.vec3(0.0, 0.0, 0.0) + if sin_half > 1.0e-10: + axis = wp.vec3(dq[0], dq[1], dq[2]) / sin_half + angle = 2.0 * wp.atan2(sin_half, dq[3]) + omega = axis * angle * inv_dt + return omegaThen both branches simply call:
omega = _quat_diff_to_angular_vel(cur_q, tgt_q, inv_dt)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1975 - 2018, The quaternion-difference-to-angular-velocity computation is duplicated in the FREE angular part and the BALL branch; extract it into a `@wp.func` named something like _quat_diff_to_angular_vel(cur_q, tgt_q, inv_dt) that returns a wp.vec3 omega computed from dq = tgt_q * wp.quat_inverse(cur_q) (handle dq[3]<0, compute sin_half, axis, angle, omega or zero), then replace both blocks to call omega = _quat_diff_to_angular_vel(cur_q, tgt_q, inv_dt) and assign omega components to joint_qd at the existing indices (wqd_i+3..+5 for FREE angular part and wqd_i+0..+2 for BALL).
1946-1953: VELOCITY mode readsdof_countentries from coordinate-indexed offset — works by convention but undocumented in the kernel.For VELOCITY mode, the kernel reads
kinematic_target[wq_i + i](coordinate-space offset) but loops only overdof_countelements. For FREE (7 coords, 6 DOFs) and BALL (4 coords, 3 DOFs) joints this silently ignores the trailing quaternion slot(s). The docstring mentions the convention but a brief inline comment inside the VELOCITY branch would help future readers understand whywq_i(notwqd_i) is used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1946 - 1953, In the VELOCITY branch of the kernel (the code using KinematicMode.VELOCITY), add a concise inline comment explaining that kinematic_target is indexed in coordinate space (use of joint_q_start/wq_i) and that we intentionally copy only dof_count entries into joint_qd (skipping trailing quaternion padding slots for FREE/BALL joints); reference the symbols joint_q_start, joint_qd_start, kinematic_target, dof_count, and joint_dof_dim so future readers understand why wq_i (not wqd_i) is used and why the loop length is dof_count.newton/examples/basic/example_basic_kinematic_platform.py (2)
238-243:test_final()validates bodies above ground — reasonable but minimal.The threshold
z > -0.5is generous. Given the platform starts at z=0.05 and creatures are dropped from z=1.0, consider also asserting that the platform has nonzero angular velocity (or has rotated), which would confirm the kinematic VELOCITY mode is actually working. This would make the test more meaningful for regression detection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 238 - 243, The test_final currently only checks heights; enhance it to also assert the kinematic platform is actually rotating by checking the platform's angular velocity in the final state: after retrieving body_q from self.state_0, read the corresponding angular velocity vector (e.g., self.state_0.body_w) for the platform body index (use self.model to identify the platform body) and assert its magnitude is above a small threshold (not zero) to confirm VELOCITY mode produced rotation; update test_final to include this additional assertion referencing test_final, self.state_0, self.model, and body_w.
170-186: Redundant intermediatewp.arraycreation at Line 173.The
wp.arraycreated at Line 173 is immediately overwritten at Line 186 after further modifications to the sametargetnumpy array. The intermediate GPU allocation is wasted.♻️ Proposed fix — set all targets then create the array once
# Set the platform angular velocity target platform_q_start = int(self.model.joint_q_start.numpy()[platform_joint]) target = self.control.kinematic_target.numpy() target[platform_q_start + 5] = PLATFORM_ANGULAR_VEL - self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) # Collect hinge q_start indices for the kernel q_starts = self.model.joint_q_start.numpy() hinge_q_starts = [int(q_starts[j]) for j in self.hinge_joints] self.hinge_q_starts_wp = wp.array(hinge_q_starts, dtype=wp.int32, device=self.model.device) self.sim_time_wp = wp.zeros(1, dtype=wp.float32, device=self.model.device) # Initialize hinge position targets to match t=0 values (avoid jump on first step) for ci, qs in enumerate(hinge_q_starts): phase = ci * 0.5 target[qs] = HINGE_AMPLITUDE * math.sin(phase) self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 170 - 186, The code creates a temporary GPU array for self.control.kinematic_target early then immediately overwrites it after further in-place edits to the numpy `target`, wasting an allocation; fix by removing the intermediate wp.array creation (the assignment at the place where `self.control.kinematic_target = wp.array(target, ...)` is first done) and instead perform all modifications on `target` (including setting PLATFORM_ANGULAR_VEL via platform_q_start and the hinge loop using `hinge_q_starts`) and then create the wp.array once at the end to assign `self.control.kinematic_target`; keep usage of symbols platform_q_start, self.control.kinematic_target, hinge_q_starts_wp, self.hinge_joints, and self.sim_time_wp unchanged.newton/_src/sim/model.py (1)
819-833: VELOCITY-mode zeroing uses DOF count but indexes into coordinate space — correct for the kernel but fragile.The loop at Line 832 zeros
dof_dims[j, 0] + dof_dims[j, 1]entries starting atq_starts[j]. For FREE joints (7 coords, 6 DOFs) and BALL joints (4 coords, 3 DOFs), this leaves trailing coordinate slots (the quaternionwcomponent) with their originaljoint_qvalue. This is harmless today becauseapply_kinematic_target_kernelonly readsdof_countelements for VELOCITY mode, but the mismatch between the indexing space (coordinate-indexed array) and the count used (DOF count) is subtle and could become a source of bugs if the kernel or another consumer changes.Consider adding a brief inline comment clarifying why DOF count (not coord count) is used here, e.g.:
# Zero only DOF-count entries: the kernel reads dof_count values from q_start offset in VELOCITY mode target_np[qs : qs + dof_dims[j, 0] + dof_dims[j, 1]] = 0.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 819 - 833, The slice that zeroes velocity joints in the joint initialization uses DOF count (q_starts and dof_dims) while indexing into coordinate space, which is intentional but subtle; update the block around target_np, q_starts and dof_dims (the loop over vel_joints in the joint_count/joint_kinematic_mode branch that sets target_np[qs:qs + dof_dims[j,0] + dof_dims[j,1]] = 0.0) to include a concise inline comment explaining that only DOF-count entries are zeroed because apply_kinematic_target_kernel (and other consumers) read dof_count values from the q_start offset in VELOCITY mode, so trailing coordinate slots (e.g., quaternion w) are intentionally left unchanged; do not change the slicing logic unless you also update the kernel/consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/sim/builder.py`:
- Around line 812-813: collapse_fixed_joints rebuilds per-joint arrays but
currently drops self.joint_kinematic_mode, causing misaligned kinematic flags;
update collapse_fixed_joints to save the current joint_kinematic_mode into the
joint_data temporary structure (alongside other per-joint fields), clear
self.joint_kinematic_mode before rebuilding, and when iterating retained joints
re-append the saved mode for each retained joint back into
self.joint_kinematic_mode so the per-joint KinematicMode stays in sync with
other rebuilt arrays.
- Around line 2869-2877: The code currently mutates caller-supplied
JointDofConfig instances by setting ax.armature = KINEMATIC_ARMATURE for
kinematic joints; instead, avoid in-place mutation by leaving JointDofConfig
objects untouched and applying the kinematic armature only to the local per-DOF
armature values you append. In practice, in the block that handles kinematic
joint overrides (referencing joint_kinematic_mode, KINEMATIC_ARMATURE,
linear_axes, angular_axes), compute the armature value as (KINEMATIC_ARMATURE if
kinematic else ax.armature) when constructing/adding the per‑DOF armature
entries rather than assigning back to ax.armature, or create a shallow copy of
each JointDofConfig and modify the copy before appending.
In `@newton/_src/sim/control.py`:
- Around line 68-77: Update the docstring for the attribute kinematic_target to
include explicit SI units: state that in KinematicMode.VELOCITY the stored
values are joint velocities in [m/s or rad/s, depending on joint type] (indexed
by joint_q_start and using q_start layout) and in KinematicMode.POSITION the
stored values are joint positions in [m or rad, depending on joint type] using
the same layout as joint_q; keep the rest of the description intact and ensure
the unit notation is inline and brief next to each mode mention.
In `@newton/_src/sim/joints.py`:
- Around line 194-218: The KinematicMode enum docstring is not in Google style;
update the docstring for KinematicMode to follow Google-style conventions by
providing a short summary line, a longer description, and explicit sections
(e.g., Attributes or Values) describing each enum member (NONE, VELOCITY,
POSITION) and clarifying how kinematic_target is indexed (reference
joint_q_start) and the difference between VELOCITY and POSITION semantics
(per-DOF velocity vs per-coordinate position and computation of velocity as
(target - current) / dt). Keep it concise, use proper section headers (Args not
needed for enums; use “Attributes” or “Values”), and ensure references to
joint_q_start and kinematic_target remain.
In `@newton/_src/sim/model.py`:
- Line 822: The import line includes an unnecessary noqa directive; remove the
trailing " # noqa: PLC0415" from the from .joints import KinematicMode statement
in model.py so the import reads simply "from .joints import KinematicMode" (no
other changes).
In `@newton/_src/solvers/mujoco/solver_mujoco.py`:
- Around line 2110-2133: The code uses a non-existent attribute
self.model.num_worlds when computing joints_per_world and launching
apply_kinematic_target_kernel; replace both occurrences of self.model.num_worlds
with the correct attribute self.model.world_count (so joints_per_world =
self.model.joint_count // self.model.world_count and
dim=(self.model.world_count, joints_per_world) in the wp.launch call) to match
the Model API and avoid AttributeError when applying kinematic targets.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 66-192: Add the new example example_basic_kinematic_platform to
the README Basic Examples table: create a row linking the module
newton.examples.basic.example_basic_kinematic_platform (GitHub file), reference
the screenshot image example_basic_kinematic_platform.jpg in the examples
images, and add the run command entry uv run -m newton.examples
basic_kinematic_platform; update the table following the existing pattern used
by other examples so the module link, screenshot, and command match formatting
and order.
- Line 190: The example directly mutates the viewer's private attribute
self.viewer._paused; add a public API on the viewer (e.g., def pause(self)
and/or def set_paused(self, value: bool)) and use it instead (replace
self.viewer._paused = True with self.viewer.pause() or
self.viewer.set_paused(True)), or alternatively remove the direct mutation and
refactor the example to rely on existing viewer methods such as is_paused();
implement the new public method(s) on the viewer class and update this example
to call them.
---
Outside diff comments:
In `@newton/_src/sim/builder.py`:
- Around line 2709-2720: The public method add_body needs its docstring updated
to document the new kinematic: KinematicMode parameter—add a Google-style Args
entry describing KinematicMode (enumeration meaning), the default
(KinematicMode.NONE), and a concise note how kinematic differs from a massless
body (kinematic controls motion semantics vs massless affecting dynamics
calculations); apply the same docstring addition to all other public methods in
this module that accept kinematic, ensure return value and behavior notes remain
accurate, and then run docs/generate_api.py to regenerate the API docs.
---
Nitpick comments:
In `@newton/_src/sim/model.py`:
- Around line 819-833: The slice that zeroes velocity joints in the joint
initialization uses DOF count (q_starts and dof_dims) while indexing into
coordinate space, which is intentional but subtle; update the block around
target_np, q_starts and dof_dims (the loop over vel_joints in the
joint_count/joint_kinematic_mode branch that sets target_np[qs:qs +
dof_dims[j,0] + dof_dims[j,1]] = 0.0) to include a concise inline comment
explaining that only DOF-count entries are zeroed because
apply_kinematic_target_kernel (and other consumers) read dof_count values from
the q_start offset in VELOCITY mode, so trailing coordinate slots (e.g.,
quaternion w) are intentionally left unchanged; do not change the slicing logic
unless you also update the kernel/consumers.
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1975-2018: The quaternion-difference-to-angular-velocity
computation is duplicated in the FREE angular part and the BALL branch; extract
it into a `@wp.func` named something like _quat_diff_to_angular_vel(cur_q, tgt_q,
inv_dt) that returns a wp.vec3 omega computed from dq = tgt_q *
wp.quat_inverse(cur_q) (handle dq[3]<0, compute sin_half, axis, angle, omega or
zero), then replace both blocks to call omega = _quat_diff_to_angular_vel(cur_q,
tgt_q, inv_dt) and assign omega components to joint_qd at the existing indices
(wqd_i+3..+5 for FREE angular part and wqd_i+0..+2 for BALL).
- Around line 1946-1953: In the VELOCITY branch of the kernel (the code using
KinematicMode.VELOCITY), add a concise inline comment explaining that
kinematic_target is indexed in coordinate space (use of joint_q_start/wq_i) and
that we intentionally copy only dof_count entries into joint_qd (skipping
trailing quaternion padding slots for FREE/BALL joints); reference the symbols
joint_q_start, joint_qd_start, kinematic_target, dof_count, and joint_dof_dim so
future readers understand why wq_i (not wqd_i) is used and why the loop length
is dof_count.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 238-243: The test_final currently only checks heights; enhance it
to also assert the kinematic platform is actually rotating by checking the
platform's angular velocity in the final state: after retrieving body_q from
self.state_0, read the corresponding angular velocity vector (e.g.,
self.state_0.body_w) for the platform body index (use self.model to identify the
platform body) and assert its magnitude is above a small threshold (not zero) to
confirm VELOCITY mode produced rotation; update test_final to include this
additional assertion referencing test_final, self.state_0, self.model, and
body_w.
- Around line 170-186: The code creates a temporary GPU array for
self.control.kinematic_target early then immediately overwrites it after further
in-place edits to the numpy `target`, wasting an allocation; fix by removing the
intermediate wp.array creation (the assignment at the place where
`self.control.kinematic_target = wp.array(target, ...)` is first done) and
instead perform all modifications on `target` (including setting
PLATFORM_ANGULAR_VEL via platform_q_start and the hinge loop using
`hinge_q_starts`) and then create the wp.array once at the end to assign
`self.control.kinematic_target`; keep usage of symbols platform_q_start,
self.control.kinematic_target, hinge_q_starts_wp, self.hinge_joints, and
self.sim_time_wp unchanged.
newton/_src/sim/model.py
Outdated
|
|
||
| # Initialize kinematic_target from joint_q, zeroing velocity-mode entries | ||
| if self.joint_count and self.joint_kinematic_mode is not None: | ||
| from .joints import KinematicMode # noqa: PLC0415 |
Contributor
There was a problem hiding this comment.
Remove unused noqa directive.
Static analysis (Ruff RUF100) flags that PLC0415 is not enabled, so the noqa comment has no effect.
- from .joints import KinematicMode # noqa: PLC0415
+ from .joints import KinematicMode📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from .joints import KinematicMode # noqa: PLC0415 | |
| from .joints import KinematicMode |
🧰 Tools
🪛 Ruff (0.15.1)
[warning] 822-822: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/_src/sim/model.py` at line 822, The import line includes an
unnecessary noqa directive; remove the trailing " # noqa: PLC0415" from the from
.joints import KinematicMode statement in model.py so the import reads simply
"from .joints import KinematicMode" (no other changes).
Comment on lines
66
to
192
| class Example: | ||
| def __init__(self, viewer, args=None): | ||
| self.fps = 100 | ||
| self.frame_dt = 1.0 / self.fps | ||
| self.sim_time = 0.0 | ||
| self.sim_substeps = 10 | ||
| self.sim_dt = self.frame_dt / self.sim_substeps | ||
|
|
||
| self.viewer = viewer | ||
| self.args = args | ||
|
|
||
| builder = newton.ModelBuilder() | ||
|
|
||
| # -- Kinematic rotating platform -- | ||
| platform_body = builder.add_link( | ||
| xform=wp.transform(p=wp.vec3(0.0, 0.0, 0.05), q=wp.quat_identity()), | ||
| mass=10.0, | ||
| armature=0.5, | ||
| ) | ||
| builder.add_shape_box( | ||
| platform_body, | ||
| hx=2.0, | ||
| hy=2.0, | ||
| hz=0.05, | ||
| cfg=newton.ModelBuilder.ShapeConfig(mu=1.0), | ||
| ) | ||
| platform_joint = builder.add_joint_free( | ||
| platform_body, | ||
| kinematic=KinematicMode.VELOCITY, | ||
| ) | ||
| builder.add_articulation([platform_joint], key="platform") | ||
|
|
||
| # -- Two-link capsule creatures dropped on the platform -- | ||
|
|
||
| self.hinge_joints = [] | ||
| capsule_radius = 0.08 | ||
| capsule_half_length = 0.2 | ||
| drop_height = 1.0 | ||
| spawn_radius = 0.8 | ||
| # Capsule shape extends along Z by default; rotate to extend along X (body-local) | ||
| capsule_rot = wp.quat_from_axis_angle(wp.vec3(0.0, 1.0, 0.0), math.pi * 0.5) | ||
|
|
||
| for ci in range(NUM_CREATURES): | ||
| angle = 2.0 * math.pi * ci / NUM_CREATURES | ||
| # Orient each creature radially outward | ||
| body_rot = wp.quat_from_axis_angle(wp.vec3(0.0, 0.0, 1.0), float(angle)) | ||
| dx = math.cos(angle) | ||
| dy = math.sin(angle) | ||
|
|
||
| # First capsule (inner) | ||
| p0 = wp.vec3(spawn_radius * dx, spawn_radius * dy, drop_height) | ||
| b0 = builder.add_link(xform=wp.transform(p=p0, q=body_rot), armature=0.5) | ||
| builder.add_shape_capsule( | ||
| b0, | ||
| radius=capsule_radius, | ||
| half_height=capsule_half_length, | ||
| xform=wp.transform(q=capsule_rot), | ||
| ) | ||
|
|
||
| j_free = builder.add_joint_free(b0) | ||
|
|
||
| # Second capsule (outer), offset along the radial direction | ||
| offset = capsule_half_length * 2.0 | ||
| p1 = wp.vec3((spawn_radius + offset) * dx, (spawn_radius + offset) * dy, drop_height) | ||
| b1 = builder.add_link(xform=wp.transform(p=p1, q=body_rot), armature=0.5) | ||
| builder.add_shape_capsule( | ||
| b1, | ||
| radius=capsule_radius, | ||
| half_height=capsule_half_length, | ||
| xform=wp.transform(q=capsule_rot), | ||
| ) | ||
|
|
||
| # Hinge between the two capsules (along body-local X which points radially) | ||
| j_hinge = builder.add_joint_revolute( | ||
| parent=b0, | ||
| child=b1, | ||
| axis=wp.vec3(0.0, 0.0, 1.0), | ||
| parent_xform=wp.transform(p=wp.vec3(capsule_half_length, 0.0, 0.0)), | ||
| child_xform=wp.transform(p=wp.vec3(-capsule_half_length, 0.0, 0.0)), | ||
| kinematic=KinematicMode.POSITION, | ||
| ) | ||
| self.hinge_joints.append(j_hinge) | ||
|
|
||
| # Set initial hinge angle to match the t=0 target (avoid velocity spike) | ||
| phase = ci * 0.5 | ||
| hinge_q_start = builder.joint_q_start[j_hinge] | ||
| builder.joint_q[hinge_q_start] = HINGE_AMPLITUDE * math.sin(phase) | ||
|
|
||
| builder.add_articulation([j_free, j_hinge]) | ||
|
|
||
| # Ground plane | ||
| builder.add_ground_plane() | ||
|
|
||
| self.model = builder.finalize() | ||
|
|
||
| self.solver = newton.solvers.SolverMuJoCo(self.model, njmax=200) | ||
|
|
||
| self.state_0 = self.model.state() | ||
| self.state_1 = self.model.state() | ||
| self.control = self.model.control() | ||
|
|
||
| newton.eval_fk(self.model, self.model.joint_q, self.model.joint_qd, self.state_0) | ||
|
|
||
| # Set the platform angular velocity target | ||
| platform_q_start = int(self.model.joint_q_start.numpy()[platform_joint]) | ||
| target = self.control.kinematic_target.numpy() | ||
| target[platform_q_start + 5] = PLATFORM_ANGULAR_VEL | ||
| self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| # Collect hinge q_start indices for the kernel | ||
| q_starts = self.model.joint_q_start.numpy() | ||
| hinge_q_starts = [int(q_starts[j]) for j in self.hinge_joints] | ||
| self.hinge_q_starts_wp = wp.array(hinge_q_starts, dtype=wp.int32, device=self.model.device) | ||
|
|
||
| self.sim_time_wp = wp.zeros(1, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| # Initialize hinge position targets to match t=0 values (avoid jump on first step) | ||
| for ci, qs in enumerate(hinge_q_starts): | ||
| phase = ci * 0.5 | ||
| target[qs] = HINGE_AMPLITUDE * math.sin(phase) | ||
| self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| self.viewer.set_model(self.model) | ||
|
|
||
| self.viewer._paused = True | ||
|
|
||
| self.capture() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'kinematic_platform|basic_kinematic' README.mdRepository: newton-physics/newton
Length of output: 47
🏁 Script executed:
# First, verify README.md exists and check its content around examples
head -100 README.md | cat -nRepository: newton-physics/newton
Length of output: 5875
🏁 Script executed:
# Also search for other example registrations to understand the pattern
rg -n 'uv run.*example|example_' README.md | head -20Repository: newton-physics/newton
Length of output: 2170
Add this example to README.md with a screenshot and uv run command entry in the Basic Examples table.
Per coding guidelines, all Newton examples must be registered in README.md. The example example_basic_kinematic_platform.py is not currently listed. Add it to the Basic Examples table following the existing pattern: GitHub file link, screenshot image (docs/images/examples/example_basic_kinematic_platform.jpg), and uv run -m newton.examples basic_kinematic_platform command.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 66 -
192, Add the new example example_basic_kinematic_platform to the README Basic
Examples table: create a row linking the module
newton.examples.basic.example_basic_kinematic_platform (GitHub file), reference
the screenshot image example_basic_kinematic_platform.jpg in the examples
images, and add the run command entry uv run -m newton.examples
basic_kinematic_platform; update the table following the existing pattern used
by other examples so the module link, screenshot, and command match formatting
and order.
|
|
||
| self.viewer.set_model(self.model) | ||
|
|
||
| self.viewer._paused = True |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how _paused is used across other examples
rg -n '_paused' --glob 'newton/examples/**/*.py' -C1Repository: newton-physics/newton
Length of output: 523
🏁 Script executed:
#!/bin/bash
# Search for viewer pause/unpause methods in the codebase
rg -n 'def (pause|unpause|set_pause)' --glob '**/*.py' -C 2
# Also check for pause as a property
rg -n '@property' --glob '**/*.py' -A 2 | grep -A 2 'pause'
# Look for viewer class definition to see public API
fd -e py -name '*viewer*' | head -20Repository: newton-physics/newton
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Find viewer-related files
fd viewer --type f
# Search for pause-related methods in the codebase
rg 'def.*pause' --glob '**/*.py' -B 1 -A 3
# Check for property definitions related to pause
rg '@property' --glob '**/*.py' -A 2 | grep -B 2 -A 2 'pause'Repository: newton-physics/newton
Length of output: 1240
🏁 Script executed:
#!/bin/bash
# Check the viewer class public API
cat -n newton/_src/viewer/viewer.py | head -100
# Search for pause-related methods
rg 'def.*pause|_paused' newton/_src/viewer/viewer.py -B 1 -A 5
# Also check viewer_gl.py for pause methods
rg 'def.*pause|_paused' newton/_src/viewer/viewer_gl.py -B 1 -A 5Repository: newton-physics/newton
Length of output: 5617
🏁 Script executed:
#!/bin/bash
# Check the context around line 190 in the example file
cat -n newton/examples/basic/example_basic_kinematic_platform.py | sed -n '180,200p'
# Check if there are other examples that set _paused
rg 'viewer\._paused\s*=' --glob 'newton/examples/**/*.py'Repository: newton-physics/newton
Length of output: 1043
🏁 Script executed:
#!/bin/bash
# Check if there's a public pause/unpause method anywhere in viewer classes
rg 'def (pause|unpause|set_paused)' newton/_src/viewer/ -A 5Repository: newton-physics/newton
Length of output: 47
Add public API method to viewer for externally setting pause state, or remove direct access to private _paused attribute.
The code directly accesses self.viewer._paused = True at line 190, which is a private attribute. No public method exists in the viewer API to set the pause state externally (only is_paused() getter is available). This pattern is not used in other examples. Either add a public method like pause() or set_paused() to the viewer API, or refactor the code to avoid accessing the private attribute.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/examples/basic/example_basic_kinematic_platform.py` at line 190, The
example directly mutates the viewer's private attribute self.viewer._paused; add
a public API on the viewer (e.g., def pause(self) and/or def set_paused(self,
value: bool)) and use it instead (replace self.viewer._paused = True with
self.viewer.pause() or self.viewer.set_paused(True)), or alternatively remove
the direct mutation and refactor the example to rely on existing viewer methods
such as is_paused(); implement the new public method(s) on the viewer class and
update this example to call them.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/sim/builder.py (1)
2710-2770:⚠️ Potential issue | 🟡 MinorDocument the new
kinematicparameter in the builder APIs.This argument is user‑facing but missing from the docstring here (and the joint helpers). Please add a short description (e.g., MuJoCo‑only kinematic control, armature override behavior).
✍️ Suggested docstring update (example for add_body)
@@ - lock_inertia: If True, prevents subsequent shape additions from modifying this body's mass, + lock_inertia: If True, prevents subsequent shape additions from modifying this body's mass, center of mass, or inertia. This does not affect merging behavior in :meth:`collapse_fixed_joints`, which always accumulates mass and inertia across merged bodies. + kinematic: Kinematic mode for the auto-created free joint (MuJoCo-only). When not NONE, the joint + is driven by ``control.kinematic_target`` and its armature is overridden for kinematic behavior.As per coding guidelines: "Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 2710 - 2770, The docstring for the body-builder method is missing documentation for the new kinematic parameter; update the docstring for this method (the function that calls add_link/add_joint_free/add_articulation) to include a short Google-style description of the kinematic parameter (e.g., "kinematic: KinematicMode flag controlling MuJoCo-only kinematic control; when mass==0 the body is treated as kinematic; also note armature/override behavior if applicable"). Also add matching docstring entries for the joint helper(s) referenced here—specifically add_joint_free (and any other joint_* helpers that accept kinematic) so the parameter is documented consistently across builder APIs; reference the symbol names add_link, add_joint_free, and add_articulation when making the edits.
🧹 Nitpick comments (5)
newton/_src/sim/model.py (1)
822-822: Remove unusednoqadirective.Ruff reports
PLC0415is not enabled, so the# noqa: PLC0415comment is unnecessary.Proposed fix
- from .joints import KinematicMode # noqa: PLC0415 + from .joints import KinematicMode🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` at line 822, The import line "from .joints import KinematicMode" contains an unnecessary "# noqa: PLC0415" directive; remove the "# noqa: PLC0415" suffix so the import reads simply "from .joints import KinematicMode" (locate this change near the import statement referencing KinematicMode in model.py).newton/examples/basic/example_basic_kinematic_platform.py (1)
238-243:test_finalthreshold of-0.5is very permissive.The ground plane is at z=0 and the platform is at z=0.05. A threshold of z > -0.5 allows bodies to be well below the ground without failing. Consider tightening to something like z > -0.1 for a more meaningful assertion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 238 - 243, The test_final assertion uses an overly permissive threshold (-0.5) allowing bodies to be far below the ground; update the check in test_final to use a tighter threshold (e.g., z > -0.1) when iterating over self.model.body_count and inspecting positions from self.state_0.body_q to ensure bodies remain at or near the expected ground/platform height.newton/_src/solvers/mujoco/kernels.py (2)
1950-1953: VELOCITY mode reads fromkinematic_target[wq_i]— confirm the coordinate/DOF offset mapping is correct for free/ball joints.For VELOCITY mode,
dof_countvelocity values are read starting atwq_i(ajoint_q_startoffset into the coord-sizedkinematic_targetarray). This works because the model initialization (model.py Lines 830–832) zeroes exactlydof_dim[0]+dof_dim[1]entries atq_startfor velocity-mode joints, leaving extra coord slots (e.g., the 7th for free joints) as-is.This is correct and consistent with the documented convention, but it's a subtle mapping worth a brief inline comment for maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1950 - 1953, Add a brief inline comment in the VELOCITY branch of the kinematic update explaining why we read velocities from kinematic_target[wq_i] rather than wqd_i for free/ball joints: note that model initialization zeroes the first dof_dim[0]+dof_dim[1] coord slots at q_start for velocity-mode joints so the velocity values occupy the wq_i offset while joint_qd is written at wqd_i; update the block around the symbols mode, KinematicMode.VELOCITY, joint_qd, kinematic_target, wqd_i, wq_i, and dof_count to document this mapping for maintainability.
1920-2023: Duplicated quaternion-to-angular-velocity logic for FREE and BALL paths.The axis-angle extraction from quaternion difference (shorter-path fix, sin_half computation, atan2) is duplicated between the FREE branch (Lines 1975–1993) and the BALL branch (Lines 2004–2018). Consider extracting a
@wp.funchelper. That said, in GPU kernels some duplication is tolerable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1920 - 2023, The FREE and BALL branches in apply_kinematic_target_kernel duplicate the quaternion-difference→angular-velocity logic; refactor by extracting that logic into a `@wp.func` helper (e.g., quat_diff_to_omega(cur_q: wp.quat, tgt_q: wp.quat, inv_dt: float) -> wp.vec3) that performs dq = tgt*inv(cur), shorter-path sign flip, sin_half/axis/angle/omega computation and returns omega, then call this helper from both the FREE (replace the manual quaternion block computing omega and writing joint_qd[wqd_i+3..5]) and BALL (replace the identical block and write joint_qd[wqd_i+0..2]) branches so the kernel body stays concise and identical behavior is preserved.newton/tests/test_mujoco_solver.py (1)
6243-6264: Avoid hard-coded joint index for armature lookup.Using
j2keeps the test robust if joint ordering changes.♻️ Suggested change
- j2_qd_start = int(model.joint_qd_start.numpy()[1]) + j2_qd_start = int(model.joint_qd_start.numpy()[j2])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/tests/test_mujoco_solver.py` around lines 6243 - 6264, The test uses a hard-coded joint index (1) when checking the armature; use the j2 identifier returned from add_joint_revolute to look up the joint's qd start and armature entry instead. Replace the literal index in the armature/qd-start lookup (model.joint_qd_start.numpy()[1] and armature[...] ) with the j2-based index (e.g., int(model.joint_qd_start.numpy()[j2])) so the assertions reference the joint created as j2 and remain correct if joint ordering changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/sim/builder.py`:
- Around line 812-813: The per-joint list self.joint_kinematic_mode isn't being
updated when collapse_fixed_joints() rebuilds joint arrays, leaving stale
entries and a mismatch with self.joint_count; update collapse_fixed_joints() to
capture the original per-joint kinematic modes, rebuild a new
joint_kinematic_mode list alongside the other joint arrays (same ordering and
filtering logic used for joint_indices/joint_names/etc.), and assign it back to
self.joint_kinematic_mode so its length and indexing match the rebuilt joints;
ensure the same fix is applied in any other collapse/folding routines mentioned
(e.g., the code region around line 8960) that rebuild joint arrays.
- Around line 2869-2877: The code currently mutates the armature on the passed
JointDofConfig axis objects (linear_axes/angular_axes) when kinematic, which can
leak into later uses; instead, compute and append a local armature value into
self.joint_armature without changing ax.armature. In the block where you check
kinematic != KinematicMode.NONE, set a local variable (e.g., armature =
KINEMATIC_ARMATURE if kinematic else ax.armature) and append that to
self.joint_armature for each axis rather than assigning to ax.armature; ensure
joint_kinematic_mode remains unchanged and do not modify the original
JointDofConfig objects.
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1995-2018: The BALL joint kinematic target computes omega in the
parent/body frame (dq = tgt_q * wp.quat_inverse(cur_q)) but writes it directly
into joint_qd; modify apply_kinematic_target_kernel to rotate omega into world
frame before storing (use the same pattern as convert_mj_coords_to_warp_kernel),
e.g. compute omega_world = wp.quat_rotate(rot, omega) (using the joint's rot)
and write omega_world into joint_qd at wqd_i+0..2 instead of omega; keep
existing cur_q, tgt_q, dq logic but perform this frame conversion prior to
assignment.
In `@newton/_src/solvers/mujoco/solver_mujoco.py`:
- Around line 2110-2132: The runtime AttributeError is caused by referencing
self.model.num_worlds which doesn't exist; update the kinematic target block to
use self.model.world_count instead: compute joints_per_world =
self.model.joint_count // self.model.world_count and pass
dim=(self.model.world_count, joints_per_world) to wp.launch; ensure any other
places in this launch call (inputs/outputs) that currently rely on num_worlds
are left unchanged except for replacing num_worlds with world_count (e.g.,
apply_kinematic_target_kernel, state_in.joint_qd, and the joints_per_world
argument).
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 66-192: Update the README.md Basic Examples table to register the
new example by adding a row that links to
newton/examples/basic/example_basic_kinematic_platform.py on GitHub, references
the screenshot image (e.g., docs/images/basic_kinematic_platform.png), and shows
the run command "uv run -m newton.examples basic_kinematic_platform"; follow the
existing table pattern (columns: Example, Screenshot, Run) used by other entries
so formatting and ordering are consistent. Ensure the link points to the raw
GitHub file path used by other examples and add the screenshot file to the
docs/images directory if it does not already exist. Verify the table renders
correctly in README.md.
In `@newton/tests/test_mujoco_solver.py`:
- Around line 6085-6103: The test docstring for
test_kinematic_mode_builder_sets_armature is a one-liner and must be converted
to a Google-style multi-line docstring that clearly describes the test behavior
and verifies kinematic joints get high armature on all DOFs; update the
docstring above test_kinematic_mode_builder_sets_armature to a Google-style
format (short summary, optional extended description, Args/Returns if
applicable) and explicitly state SI units for all physical quantities used in
the test (e.g., mass [kg] for builder.add_link, inertia [kg·m^2] for
wp.mat33(np.eye(3)), shape dimensions hx/hy/hz [m] for add_shape_box, velocities
[m/s or rad/s] and time step [s] if referenced), and mention the expected
armature magnitude with units (armature entries set to 1.0e10, include the
unit), while keeping content concise and focused on the assertions around
model.joint_armature and model.joint_kinematic_mode.
---
Outside diff comments:
In `@newton/_src/sim/builder.py`:
- Around line 2710-2770: The docstring for the body-builder method is missing
documentation for the new kinematic parameter; update the docstring for this
method (the function that calls add_link/add_joint_free/add_articulation) to
include a short Google-style description of the kinematic parameter (e.g.,
"kinematic: KinematicMode flag controlling MuJoCo-only kinematic control; when
mass==0 the body is treated as kinematic; also note armature/override behavior
if applicable"). Also add matching docstring entries for the joint helper(s)
referenced here—specifically add_joint_free (and any other joint_* helpers that
accept kinematic) so the parameter is documented consistently across builder
APIs; reference the symbol names add_link, add_joint_free, and add_articulation
when making the edits.
---
Nitpick comments:
In `@newton/_src/sim/model.py`:
- Line 822: The import line "from .joints import KinematicMode" contains an
unnecessary "# noqa: PLC0415" directive; remove the "# noqa: PLC0415" suffix so
the import reads simply "from .joints import KinematicMode" (locate this change
near the import statement referencing KinematicMode in model.py).
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1950-1953: Add a brief inline comment in the VELOCITY branch of
the kinematic update explaining why we read velocities from
kinematic_target[wq_i] rather than wqd_i for free/ball joints: note that model
initialization zeroes the first dof_dim[0]+dof_dim[1] coord slots at q_start for
velocity-mode joints so the velocity values occupy the wq_i offset while
joint_qd is written at wqd_i; update the block around the symbols mode,
KinematicMode.VELOCITY, joint_qd, kinematic_target, wqd_i, wq_i, and dof_count
to document this mapping for maintainability.
- Around line 1920-2023: The FREE and BALL branches in
apply_kinematic_target_kernel duplicate the
quaternion-difference→angular-velocity logic; refactor by extracting that logic
into a `@wp.func` helper (e.g., quat_diff_to_omega(cur_q: wp.quat, tgt_q: wp.quat,
inv_dt: float) -> wp.vec3) that performs dq = tgt*inv(cur), shorter-path sign
flip, sin_half/axis/angle/omega computation and returns omega, then call this
helper from both the FREE (replace the manual quaternion block computing omega
and writing joint_qd[wqd_i+3..5]) and BALL (replace the identical block and
write joint_qd[wqd_i+0..2]) branches so the kernel body stays concise and
identical behavior is preserved.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 238-243: The test_final assertion uses an overly permissive
threshold (-0.5) allowing bodies to be far below the ground; update the check in
test_final to use a tighter threshold (e.g., z > -0.1) when iterating over
self.model.body_count and inspecting positions from self.state_0.body_q to
ensure bodies remain at or near the expected ground/platform height.
In `@newton/tests/test_mujoco_solver.py`:
- Around line 6243-6264: The test uses a hard-coded joint index (1) when
checking the armature; use the j2 identifier returned from add_joint_revolute to
look up the joint's qd start and armature entry instead. Replace the literal
index in the armature/qd-start lookup (model.joint_qd_start.numpy()[1] and
armature[...] ) with the j2-based index (e.g.,
int(model.joint_qd_start.numpy()[j2])) so the assertions reference the joint
created as j2 and remain correct if joint ordering changes.
Comment on lines
66
to
192
| class Example: | ||
| def __init__(self, viewer, args=None): | ||
| self.fps = 100 | ||
| self.frame_dt = 1.0 / self.fps | ||
| self.sim_time = 0.0 | ||
| self.sim_substeps = 10 | ||
| self.sim_dt = self.frame_dt / self.sim_substeps | ||
|
|
||
| self.viewer = viewer | ||
| self.args = args | ||
|
|
||
| builder = newton.ModelBuilder() | ||
|
|
||
| # -- Kinematic rotating platform -- | ||
| platform_body = builder.add_link( | ||
| xform=wp.transform(p=wp.vec3(0.0, 0.0, 0.05), q=wp.quat_identity()), | ||
| mass=10.0, | ||
| armature=0.5, | ||
| ) | ||
| builder.add_shape_box( | ||
| platform_body, | ||
| hx=2.0, | ||
| hy=2.0, | ||
| hz=0.05, | ||
| cfg=newton.ModelBuilder.ShapeConfig(mu=1.0), | ||
| ) | ||
| platform_joint = builder.add_joint_free( | ||
| platform_body, | ||
| kinematic=KinematicMode.VELOCITY, | ||
| ) | ||
| builder.add_articulation([platform_joint], key="platform") | ||
|
|
||
| # -- Two-link capsule creatures dropped on the platform -- | ||
|
|
||
| self.hinge_joints = [] | ||
| capsule_radius = 0.08 | ||
| capsule_half_length = 0.2 | ||
| drop_height = 1.0 | ||
| spawn_radius = 0.8 | ||
| # Capsule shape extends along Z by default; rotate to extend along X (body-local) | ||
| capsule_rot = wp.quat_from_axis_angle(wp.vec3(0.0, 1.0, 0.0), math.pi * 0.5) | ||
|
|
||
| for ci in range(NUM_CREATURES): | ||
| angle = 2.0 * math.pi * ci / NUM_CREATURES | ||
| # Orient each creature radially outward | ||
| body_rot = wp.quat_from_axis_angle(wp.vec3(0.0, 0.0, 1.0), float(angle)) | ||
| dx = math.cos(angle) | ||
| dy = math.sin(angle) | ||
|
|
||
| # First capsule (inner) | ||
| p0 = wp.vec3(spawn_radius * dx, spawn_radius * dy, drop_height) | ||
| b0 = builder.add_link(xform=wp.transform(p=p0, q=body_rot), armature=0.5) | ||
| builder.add_shape_capsule( | ||
| b0, | ||
| radius=capsule_radius, | ||
| half_height=capsule_half_length, | ||
| xform=wp.transform(q=capsule_rot), | ||
| ) | ||
|
|
||
| j_free = builder.add_joint_free(b0) | ||
|
|
||
| # Second capsule (outer), offset along the radial direction | ||
| offset = capsule_half_length * 2.0 | ||
| p1 = wp.vec3((spawn_radius + offset) * dx, (spawn_radius + offset) * dy, drop_height) | ||
| b1 = builder.add_link(xform=wp.transform(p=p1, q=body_rot), armature=0.5) | ||
| builder.add_shape_capsule( | ||
| b1, | ||
| radius=capsule_radius, | ||
| half_height=capsule_half_length, | ||
| xform=wp.transform(q=capsule_rot), | ||
| ) | ||
|
|
||
| # Hinge between the two capsules (along body-local X which points radially) | ||
| j_hinge = builder.add_joint_revolute( | ||
| parent=b0, | ||
| child=b1, | ||
| axis=wp.vec3(0.0, 0.0, 1.0), | ||
| parent_xform=wp.transform(p=wp.vec3(capsule_half_length, 0.0, 0.0)), | ||
| child_xform=wp.transform(p=wp.vec3(-capsule_half_length, 0.0, 0.0)), | ||
| kinematic=KinematicMode.POSITION, | ||
| ) | ||
| self.hinge_joints.append(j_hinge) | ||
|
|
||
| # Set initial hinge angle to match the t=0 target (avoid velocity spike) | ||
| phase = ci * 0.5 | ||
| hinge_q_start = builder.joint_q_start[j_hinge] | ||
| builder.joint_q[hinge_q_start] = HINGE_AMPLITUDE * math.sin(phase) | ||
|
|
||
| builder.add_articulation([j_free, j_hinge]) | ||
|
|
||
| # Ground plane | ||
| builder.add_ground_plane() | ||
|
|
||
| self.model = builder.finalize() | ||
|
|
||
| self.solver = newton.solvers.SolverMuJoCo(self.model, njmax=200) | ||
|
|
||
| self.state_0 = self.model.state() | ||
| self.state_1 = self.model.state() | ||
| self.control = self.model.control() | ||
|
|
||
| newton.eval_fk(self.model, self.model.joint_q, self.model.joint_qd, self.state_0) | ||
|
|
||
| # Set the platform angular velocity target | ||
| platform_q_start = int(self.model.joint_q_start.numpy()[platform_joint]) | ||
| target = self.control.kinematic_target.numpy() | ||
| target[platform_q_start + 5] = PLATFORM_ANGULAR_VEL | ||
| self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| # Collect hinge q_start indices for the kernel | ||
| q_starts = self.model.joint_q_start.numpy() | ||
| hinge_q_starts = [int(q_starts[j]) for j in self.hinge_joints] | ||
| self.hinge_q_starts_wp = wp.array(hinge_q_starts, dtype=wp.int32, device=self.model.device) | ||
|
|
||
| self.sim_time_wp = wp.zeros(1, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| # Initialize hinge position targets to match t=0 values (avoid jump on first step) | ||
| for ci, qs in enumerate(hinge_q_starts): | ||
| phase = ci * 0.5 | ||
| target[qs] = HINGE_AMPLITUDE * math.sin(phase) | ||
| self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) | ||
|
|
||
| self.viewer.set_model(self.model) | ||
|
|
||
| self.viewer._paused = True | ||
|
|
||
| self.capture() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the example is registered in README.md
rg -n "basic_kinematic_platform" README.mdRepository: newton-physics/newton
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check README.md structure and find existing example registrations
head -100 README.mdRepository: newton-physics/newton
Length of output: 5175
Add example registration to README.md.
The example is not registered in the README.md examples table. Following the established pattern in the Basic Examples section, add an entry with a link to the GitHub file, a screenshot image, and the command uv run -m newton.examples basic_kinematic_platform.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 66 -
192, Update the README.md Basic Examples table to register the new example by
adding a row that links to
newton/examples/basic/example_basic_kinematic_platform.py on GitHub, references
the screenshot image (e.g., docs/images/basic_kinematic_platform.png), and shows
the run command "uv run -m newton.examples basic_kinematic_platform"; follow the
existing table pattern (columns: Example, Screenshot, Run) used by other entries
so formatting and ordering are consistent. Ensure the link points to the raw
GitHub file path used by other examples and add the screenshot file to the
docs/images directory if it does not already exist. Verify the table renders
correctly in README.md.
newton/tests/test_mujoco_solver.py
Outdated
Comment on lines
6085
to
6103
| def test_kinematic_mode_builder_sets_armature(self): | ||
| """Test that kinematic joints get high armature on all DOFs.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([joint]) | ||
| model = builder.finalize() | ||
|
|
||
| armature = model.joint_armature.numpy() | ||
| qd_start = model.joint_qd_start.numpy()[joint] | ||
| # All 6 DOFs should have armature = 1e10 | ||
| for i in range(6): | ||
| self.assertAlmostEqual(float(armature[qd_start + i]), 1.0e10, places=0) | ||
|
|
||
| # joint_kinematic_mode should be VELOCITY | ||
| mode = model.joint_kinematic_mode.numpy()[joint] | ||
| self.assertEqual(mode, int(newton.KinematicMode.VELOCITY)) | ||
|
|
Contributor
There was a problem hiding this comment.
Update new kinematic test docstrings to Google style and include units.
These new test docstrings are one-liners and omit SI units for the physical quantities exercised (e.g., positions, angles, velocities, dt). Please expand them to Google-style docstrings and add inline unit tags like [m], [rad], [s], [rad/s] where applicable. As per coding guidelines "Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values" and "State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N], [N·m], or [m or rad, depending on joint type]); skip non-physical fields".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/tests/test_mujoco_solver.py` around lines 6085 - 6103, The test
docstring for test_kinematic_mode_builder_sets_armature is a one-liner and must
be converted to a Google-style multi-line docstring that clearly describes the
test behavior and verifies kinematic joints get high armature on all DOFs;
update the docstring above test_kinematic_mode_builder_sets_armature to a
Google-style format (short summary, optional extended description, Args/Returns
if applicable) and explicitly state SI units for all physical quantities used in
the test (e.g., mass [kg] for builder.add_link, inertia [kg·m^2] for
wp.mat33(np.eye(3)), shape dimensions hx/hy/hz [m] for add_shape_box, velocities
[m/s or rad/s] and time step [s] if referenced), and mention the expected
armature magnitude with units (armature entries set to 1.0e10, include the
unit), while keeping content concise and focused on the assertions around
model.joint_armature and model.joint_kinematic_mode.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
newton/_src/sim/model.py (2)
822-822: Remove unusednoqadirective.Static analysis (Ruff RUF100) reports that
# noqa: PLC0415is unused because the rule isn't enabled. Remove it or replace with the correct suppression if needed.- from .joints import KinematicMode # noqa: PLC0415 + from .joints import KinematicMode🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` at line 822, The trailing noqa suppression on the import is unnecessary; in the line that imports KinematicMode (from .joints import KinematicMode) remove the unused "# noqa: PLC0415" directive so the import reads normally; if you intended to suppress a different active lint (e.g., unused-import), replace it with the correct code, otherwise just delete the noqa comment.
819-833: Kinematic target initialization looks correct but consider theclone_variables=Falsepath.When
clone_variables=False, other control arrays (e.g.,joint_target_pos) use references to the model arrays instead of clones (lines 813-818). However,kinematic_targetalways allocates a newwp.arrayfrom a numpy copy regardless ofclone_variables. This is fine since there's no model-levelkinematic_targetto reference, but it's worth a brief inline comment for future maintainers who might expect symmetry with the other control attributes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 819 - 833, The kinematic_target allocation always creates a new wp.array from a numpy copy even when clone_variables=False, which can confuse future readers expecting the same reference-sharing used for other controls like joint_target_pos; add a brief inline comment next to the kinematic_target creation (near kinematic_target, clone_variables, joint_target_pos, joint_q, joint_kinematic_mode references) explaining that there is no model-level kinematic_target to reference so it must be allocated anew and that this behavior is intentional to avoid unintended aliasing with model arrays.newton/examples/basic/example_basic_kinematic_platform.py (2)
44-58: Consider usingwp.constantormath.piinstead of inline pi literal.Line 57 uses
3.14159265as an approximation of π. Warp provides math functions andmath.piis available. While the precision is sufficient for this use case, it's a minor readability improvement.- omega = 2.0 * 3.14159265 * frequency + omega = 2.0 * 3.141592653589793 * frequencyOr define a module-level constant:
PI = wp.constant(math.pi).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 44 - 58, The kernel update_hinge_targets uses an inline pi literal (3.14159265) when computing omega; replace it with a clearer constant by importing math and using math.pi or define a module-level wp.constant like PI = wp.constant(math.pi) and then compute omega = 2.0 * PI * frequency (or use math.pi directly) so the calculation in update_hinge_targets remains accurate and more readable.
238-243:test_final()could be more assertive.The test only checks
z > -0.5, which is a very loose bound. Consider also verifying that the platform body has actually rotated (e.g., checking the quaternion has changed from identity) to confirm kinematic velocity mode is working. This would catch regressions where kinematic targets are silently ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 238 - 243, The current test_final() only asserts bodies stayed above z > -0.5 which is too loose; update test_final to also assert the platform's orientation actually changed by comparing the platform body's quaternion in self.state_0.body_q to the identity quaternion (e.g., [0,0,0,1]) so we detect rotation being applied — locate test_final and body_q (self.state_0.body_q.numpy()), identify the platform body index (e.g., platform_body_index or whichever index corresponds to the platform in model.body_count) and add an assertion that the quaternion differs from identity beyond a tiny epsilon (not just that z is above a threshold).newton/_src/solvers/mujoco/kernels.py (1)
1975-1993: Consider extracting quaternion-to-angular-velocity as a@wp.func.The quaternion difference → axis-angle → angular velocity computation is duplicated for FREE (lines 1975-1993) and BALL (lines 2004-2018) joints. Extracting it into a helper function would reduce duplication and make it easier to fix bugs (like the frame issue above) in one place.
Suggested helper
`@wp.func` def quat_diff_to_angular_vel(cur_q: wp.quat, tgt_q: wp.quat, inv_dt: float) -> wp.vec3: """Compute angular velocity from quaternion difference: (tgt * inv(cur)) / dt.""" dq = tgt_q * wp.quat_inverse(cur_q) if dq[3] < 0.0: dq = wp.quat(-dq[0], -dq[1], -dq[2], -dq[3]) sin_half = wp.sqrt(dq[0] * dq[0] + dq[1] * dq[1] + dq[2] * dq[2]) if sin_half > 1.0e-10: axis = wp.vec3(dq[0], dq[1], dq[2]) / sin_half angle = 2.0 * wp.atan2(sin_half, dq[3]) return axis * angle * inv_dt return wp.vec3(0.0, 0.0, 0.0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1975 - 1993, Extract the duplicated quaternion-difference → axis-angle → angular velocity logic into a single `@wp.func` helper (e.g., quat_diff_to_angular_vel(cur_q: wp.quat, tgt_q: wp.quat, inv_dt: float) -> wp.vec3) and replace the duplicated blocks in the FREE joint code (the block that computes dq, sin_half, axis, angle, omega and writes joint_qd[wqd_i + 3..5]) and the BALL joint block with calls to this helper; ensure the helper implements the same negation-for-shorter-path (dq[3] < 0.0) and the sin_half threshold, and return a wp.vec3 omega which the callers assign into joint_qd at indices wqd_i+3..5.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@newton/_src/sim/builder.py`:
- Around line 2869-2877: The code currently mutates caller-provided
JointDofConfig by setting ax.armature = KINEMATIC_ARMATURE inside the kinematic
branch (seen in joint_kinematic_mode, KINEMATIC_ARMATURE, linear_axes,
angular_axes), which can leak the huge armature into other joints; instead stop
writing to ax.armature and compute a local joint_armature = KINEMATIC_ARMATURE
if kinematic else ax.armature (or default) and use that local variable when
constructing/assigning the joint's armature value; remove the ax.armature
assignments and ensure any place that previously relied on ax.armature for this
joint uses the new joint_armature local variable so JointDofConfig objects are
not mutated.
- Around line 812-813: collapse_fixed_joints currently rebuilds per-joint arrays
but leaves self.joint_kinematic_mode unchanged, causing desynchronization after
joints are collapsed; update collapse_fixed_joints to clear and repopulate
self.joint_kinematic_mode in the same retained-joint rebuild loop where other
per-joint arrays are rebuilt (use the same retained index mapping logic and
KinematicMode values you apply to arrays such as joint_qpos0/joint_qvel0),
ensuring joint_kinematic_mode is sliced/assigned consistently for all kept
joints and defaulted/removed for collapsed ones.
- Around line 2719-2720: The new kinematic: KinematicMode parameter on the
function (and the related add_joint and add_joint_* helpers) lacks docstring
documentation; update the Google-style docstring for this function to include an
Args entry describing kinematic (type KinematicMode, allowed values, default and
behavior/contract when set), and mirror that same description in the docstrings
of add_joint and each add_joint_* wrapper so callers understand the runtime
semantics and default; ensure the return and other existing param docs remain
unchanged and follow the existing add_body/doc style.
In `@newton/_src/sim/control.py`:
- Around line 68-77: The docstring for the attribute kinematic_target lacks SI
unit annotations; update the docstring for self.kinematic_target to state the
physical units using inline [unit] notation (e.g., "[m or rad]" per joint type
for POSITION mode and "[m/s or rad/s]" for VELOCITY mode) and mention the shape
and indexing already present; reference kinematic_target, joint_q_start,
KinematicMode.VELOCITY, KinematicMode.POSITION and clear so readers know targets
persist and how units map to the stored values.
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1995-2018: The computed omega for BALL joints (inside the jtype ==
int(JointType.BALL) branch) is currently in the parent/world frame; rotate it
into the child/body frame before writing to joint_qd: after computing omega from
dq and inv_dt, apply the inverse of cur_q to omega (e.g., use
wp.quat_rotate(wp.quat_inverse(cur_q), omega) or equivalent) and then store that
rotated vector into joint_qd[wqd_i + 0..2]; retain the existing dq, cur_q,
tgt_q, and inv_dt computations and only change the final assignment to write the
body-frame angular velocity.
In `@newton/_src/solvers/mujoco/solver_mujoco.py`:
- Around line 2110-2127: The code uses a non-existent attribute
self.model.num_worlds causing an AttributeError; update the calculation and
kernel launch to use self.model.world_count instead. Specifically, change how
joints_per_world is computed (use self.model.joint_count //
self.model.world_count) and pass self.model.world_count into the wp.launch dim
and inputs where num_worlds was used for apply_kinematic_target_kernel so the
kernel dimensions and related inputs (apply_kinematic_target_kernel,
joints_per_world, inv_dt, etc.) reference world_count consistently.
In `@newton/tests/test_mujoco_solver.py`:
- Around line 6085-6357: Several test docstrings are one-line summaries and
missing SI unit annotations; update the docstrings for the listed test functions
(e.g., test_kinematic_mode_builder_sets_armature,
test_kinematic_mode_none_no_armature_override,
test_kinematic_velocity_target_applied,
test_kinematic_position_target_computes_velocity,
test_kinematic_body_stays_in_place, test_kinematic_add_body_convenience,
test_kinematic_revolute_joint, test_kinematic_multiworld_velocity,
test_kinematic_multiworld_position) to Google-style: include a short summary
line, a blank line, and an optional Args/Returns section as appropriate (even
for tests), and annotate all physical quantities with SI units inline (e.g.,
[m], [rad], [rad/s], [m/s], [s]); ensure docstrings mention which variables have
units (dt, position/velocity/angle targets, z heights, etc.) and keep
descriptions concise and consistent across these test_* functions.
- Around line 6122-6155: The test is indexing the VELOCITY kinematic target
using joint_q_start which can mis-index DOF-specific velocity targets; update
test_kinematic_velocity_target_applied to use model.joint_qd_start (and the
joint_qd_start variable) when computing qd_start and when writing into
control.kinematic_target so the target is placed at the per-DOF velocity index
(replace uses of joint_q_start → joint_qd_start and ensure qd_start is derived
from model.joint_qd_start.numpy()[joint]); apply the same change to the other
VELOCITY-mode tests referenced (around lines 6265-6297) so all velocity-target
indexing uses joint_qd_start.
---
Nitpick comments:
In `@newton/_src/sim/model.py`:
- Line 822: The trailing noqa suppression on the import is unnecessary; in the
line that imports KinematicMode (from .joints import KinematicMode) remove the
unused "# noqa: PLC0415" directive so the import reads normally; if you intended
to suppress a different active lint (e.g., unused-import), replace it with the
correct code, otherwise just delete the noqa comment.
- Around line 819-833: The kinematic_target allocation always creates a new
wp.array from a numpy copy even when clone_variables=False, which can confuse
future readers expecting the same reference-sharing used for other controls like
joint_target_pos; add a brief inline comment next to the kinematic_target
creation (near kinematic_target, clone_variables, joint_target_pos, joint_q,
joint_kinematic_mode references) explaining that there is no model-level
kinematic_target to reference so it must be allocated anew and that this
behavior is intentional to avoid unintended aliasing with model arrays.
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1975-1993: Extract the duplicated quaternion-difference →
axis-angle → angular velocity logic into a single `@wp.func` helper (e.g.,
quat_diff_to_angular_vel(cur_q: wp.quat, tgt_q: wp.quat, inv_dt: float) ->
wp.vec3) and replace the duplicated blocks in the FREE joint code (the block
that computes dq, sin_half, axis, angle, omega and writes joint_qd[wqd_i +
3..5]) and the BALL joint block with calls to this helper; ensure the helper
implements the same negation-for-shorter-path (dq[3] < 0.0) and the sin_half
threshold, and return a wp.vec3 omega which the callers assign into joint_qd at
indices wqd_i+3..5.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 44-58: The kernel update_hinge_targets uses an inline pi literal
(3.14159265) when computing omega; replace it with a clearer constant by
importing math and using math.pi or define a module-level wp.constant like PI =
wp.constant(math.pi) and then compute omega = 2.0 * PI * frequency (or use
math.pi directly) so the calculation in update_hinge_targets remains accurate
and more readable.
- Around line 238-243: The current test_final() only asserts bodies stayed above
z > -0.5 which is too loose; update test_final to also assert the platform's
orientation actually changed by comparing the platform body's quaternion in
self.state_0.body_q to the identity quaternion (e.g., [0,0,0,1]) so we detect
rotation being applied — locate test_final and body_q
(self.state_0.body_q.numpy()), identify the platform body index (e.g.,
platform_body_index or whichever index corresponds to the platform in
model.body_count) and add an assertion that the quaternion differs from identity
beyond a tiny epsilon (not just that z is above a threshold).
newton/_src/sim/builder.py
Outdated
Comment on lines
2719
to
2720
| kinematic: KinematicMode = KinematicMode.NONE, | ||
| ) -> int: |
Contributor
There was a problem hiding this comment.
Document the new kinematic parameter.
The new parameter isn’t described in the docstrings, which makes the runtime contract unclear. Please add an Args entry (and mirror it in add_joint / add_joint_* helpers).
📄 Docstring addition (example for add_body)
@@
Args:
@@
lock_inertia: If True, prevents subsequent shape additions from modifying this body's mass,
center of mass, or inertia. This does not affect merging behavior in
:meth:`collapse_fixed_joints`, which always accumulates mass and inertia across merged bodies.
+ kinematic: Kinematic mode for the auto-created free joint. Use NONE for dynamic bodies; VELOCITY or
+ POSITION drives the joint via control.kinematic_target.As per coding guidelines: **/*.py: Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/_src/sim/builder.py` around lines 2719 - 2720, The new kinematic:
KinematicMode parameter on the function (and the related add_joint and
add_joint_* helpers) lacks docstring documentation; update the Google-style
docstring for this function to include an Args entry describing kinematic (type
KinematicMode, allowed values, default and behavior/contract when set), and
mirror that same description in the docstrings of add_joint and each add_joint_*
wrapper so callers understand the runtime semantics and default; ensure the
return and other existing param docs remain unchanged and follow the existing
add_body/doc style.
newton/tests/test_mujoco_solver.py
Outdated
Comment on lines
6085
to
6357
| def test_kinematic_mode_builder_sets_armature(self): | ||
| """Test that kinematic joints get high armature on all DOFs.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([joint]) | ||
| model = builder.finalize() | ||
|
|
||
| armature = model.joint_armature.numpy() | ||
| qd_start = model.joint_qd_start.numpy()[joint] | ||
| # All 6 DOFs should have armature = 1e10 | ||
| for i in range(6): | ||
| self.assertAlmostEqual(float(armature[qd_start + i]), 1.0e10, places=0) | ||
|
|
||
| # joint_kinematic_mode should be VELOCITY | ||
| mode = model.joint_kinematic_mode.numpy()[joint] | ||
| self.assertEqual(mode, int(newton.KinematicMode.VELOCITY)) | ||
|
|
||
| def test_kinematic_mode_none_no_armature_override(self): | ||
| """Test that non-kinematic joints keep their original armature.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body) # default kinematic=NONE | ||
| builder.add_articulation([joint]) | ||
| model = builder.finalize() | ||
|
|
||
| armature = model.joint_armature.numpy() | ||
| qd_start = model.joint_qd_start.numpy()[joint] | ||
| # create_unlimited sets armature=0.0 | ||
| for i in range(6): | ||
| self.assertAlmostEqual(float(armature[qd_start + i]), 0.0, places=5) | ||
|
|
||
| mode = model.joint_kinematic_mode.numpy()[joint] | ||
| self.assertEqual(mode, int(newton.KinematicMode.NONE)) | ||
|
|
||
| def test_kinematic_velocity_target_applied(self): | ||
| """Test that VELOCITY mode kinematic target is applied to joint_qd.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 1.0)), | ||
| mass=1.0, | ||
| inertia=wp.mat33(np.eye(3)), | ||
| ) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([joint]) | ||
| builder.add_ground_plane() | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
| newton.eval_fk(model, model.joint_q, model.joint_qd, state_0) | ||
|
|
||
| # Set angular velocity target: wz = 2.0 rad/s | ||
| q_start = int(model.joint_q_start.numpy()[joint]) | ||
| target = control.kinematic_target.numpy() | ||
| target[q_start + 5] = 2.0 # wz | ||
| control.kinematic_target = wp.array(target, dtype=wp.float32, device=model.device) | ||
|
|
||
| dt = 0.01 | ||
| solver.step(state_0, state_1, control, None, dt) | ||
|
|
||
| # Check that the output state has angular velocity close to 2.0 | ||
| qd = state_1.joint_qd.numpy() | ||
| qd_start = int(model.joint_qd_start.numpy()[joint]) | ||
| self.assertAlmostEqual(float(qd[qd_start + 5]), 2.0, delta=0.1) | ||
|
|
||
| def test_kinematic_position_target_computes_velocity(self): | ||
| """Test that POSITION mode computes velocity from target - current.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 1.0)), | ||
| mass=1.0, | ||
| inertia=wp.mat33(np.eye(3)), | ||
| ) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.POSITION) | ||
| builder.add_articulation([joint]) | ||
| builder.add_ground_plane() | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
| newton.eval_fk(model, model.joint_q, model.joint_qd, state_0) | ||
|
|
||
| # Set position target: move 0.1 in x direction from initial position | ||
| q_start = int(model.joint_q_start.numpy()[joint]) | ||
| target = control.kinematic_target.numpy() | ||
| target[q_start + 0] = 0.1 # target x = 0.1 (initial x = 0.0) | ||
| control.kinematic_target = wp.array(target, dtype=wp.float32, device=model.device) | ||
|
|
||
| dt = 0.01 | ||
| solver.step(state_0, state_1, control, None, dt) | ||
|
|
||
| # Expected velocity: (0.1 - 0.0) / 0.01 = 10.0 m/s in x | ||
| # With huge armature the actual velocity should be very close | ||
| qd = state_1.joint_qd.numpy() | ||
| qd_start = int(model.joint_qd_start.numpy()[joint]) | ||
| self.assertAlmostEqual(float(qd[qd_start + 0]), 10.0, delta=0.5) | ||
|
|
||
| def test_kinematic_body_stays_in_place(self): | ||
| """Test that a kinematic body with zero velocity target stays stationary.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 2.0)), | ||
| mass=1.0, | ||
| inertia=wp.mat33(np.eye(3)), | ||
| ) | ||
| builder.add_shape_box(body, hx=0.2, hy=0.2, hz=0.2) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([joint]) | ||
| builder.add_ground_plane() | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
| newton.eval_fk(model, model.joint_q, model.joint_qd, state_0) | ||
|
|
||
| # kinematic_target is zero for VELOCITY mode by default | ||
| # Run 100 substeps — body should resist gravity and stay at z=2.0 | ||
| dt = 0.001 | ||
| for _ in range(100): | ||
| solver.step(state_0, state_1, control, None, dt) | ||
| state_0, state_1 = state_1, state_0 | ||
|
|
||
| body_q = state_0.body_q.numpy() | ||
| z = float(body_q[body][2]) | ||
| self.assertAlmostEqual(z, 2.0, delta=0.01, msg=f"Kinematic body drifted: z={z}") | ||
|
|
||
| def test_kinematic_add_body_convenience(self): | ||
| """Test that add_body(kinematic=...) creates a kinematic free joint.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_body( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 1.0)), | ||
| mass=1.0, | ||
| kinematic=newton.KinematicMode.POSITION, | ||
| ) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| model = builder.finalize() | ||
|
|
||
| # Should have 1 joint with kinematic mode POSITION | ||
| self.assertEqual(model.joint_count, 1) | ||
| mode = model.joint_kinematic_mode.numpy()[0] | ||
| self.assertEqual(mode, int(newton.KinematicMode.POSITION)) | ||
|
|
||
| # All 6 DOFs should have high armature | ||
| armature = model.joint_armature.numpy() | ||
| for i in range(6): | ||
| self.assertAlmostEqual(float(armature[i]), 1.0e10, places=0) | ||
|
|
||
| def test_kinematic_revolute_joint(self): | ||
| """Test kinematic mode on a revolute joint.""" | ||
| builder = newton.ModelBuilder() | ||
| b1 = builder.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| b2 = builder.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| builder.add_shape_box(b1, hx=0.1, hy=0.1, hz=0.1) | ||
| builder.add_shape_box(b2, hx=0.1, hy=0.1, hz=0.1) | ||
| j1 = builder.add_joint_revolute(-1, b1, axis=(0, 0, 1)) | ||
| j2 = builder.add_joint_revolute(b1, b2, axis=(0, 0, 1), kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([j1, j2]) | ||
| model = builder.finalize() | ||
|
|
||
| # j1 should be NONE, j2 should be VELOCITY | ||
| modes = model.joint_kinematic_mode.numpy() | ||
| self.assertEqual(modes[0], int(newton.KinematicMode.NONE)) | ||
| self.assertEqual(modes[1], int(newton.KinematicMode.VELOCITY)) | ||
|
|
||
| # j2 armature should be high | ||
| armature = model.joint_armature.numpy() | ||
| j2_qd_start = int(model.joint_qd_start.numpy()[1]) | ||
| self.assertAlmostEqual(float(armature[j2_qd_start]), 1.0e10, places=0) | ||
|
|
||
| def test_kinematic_multiworld_velocity(self): | ||
| """Test kinematic velocity mode works across multiple worlds.""" | ||
| template = newton.ModelBuilder() | ||
| body = template.add_link( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 2.0)), | ||
| mass=1.0, | ||
| inertia=wp.mat33(np.eye(3)), | ||
| ) | ||
| template.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = template.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| template.add_articulation([joint]) | ||
| template.add_ground_plane() | ||
|
|
||
| num_worlds = 3 | ||
| builder = newton.ModelBuilder() | ||
| builder.replicate(template, num_worlds) | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
| newton.eval_fk(model, model.joint_q, model.joint_qd, state_0) | ||
|
|
||
| # Set different angular velocity for each world | ||
| target = control.kinematic_target.numpy() | ||
| q_starts = model.joint_q_start.numpy() | ||
| for w in range(num_worlds): | ||
| joint_idx = w # one joint per world | ||
| qs = q_starts[joint_idx] | ||
| target[qs + 5] = float(w + 1) # wz = 1, 2, 3 rad/s | ||
| control.kinematic_target = wp.array(target, dtype=wp.float32, device=model.device) | ||
|
|
||
| dt = 0.001 | ||
| for _ in range(50): | ||
| solver.step(state_0, state_1, control, None, dt) | ||
| state_0, state_1 = state_1, state_0 | ||
|
|
||
| # All bodies should stay at roughly z=2.0 | ||
| body_q = state_0.body_q.numpy() | ||
| for w in range(num_worlds): | ||
| z = float(body_q[w][2]) | ||
| self.assertAlmostEqual(z, 2.0, delta=0.05, msg=f"World {w} body drifted: z={z}") | ||
|
|
||
| def test_kinematic_multiworld_position(self): | ||
| """Test kinematic position mode works across multiple worlds.""" | ||
| template = newton.ModelBuilder() | ||
| b1 = template.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| b2 = template.add_link(mass=1.0, inertia=wp.mat33(np.eye(3))) | ||
| template.add_shape_box(b1, hx=0.1, hy=0.1, hz=0.1) | ||
| template.add_shape_box(b2, hx=0.1, hy=0.1, hz=0.1) | ||
| j1 = template.add_joint_revolute(-1, b1, axis=(0, 0, 1)) | ||
| j2 = template.add_joint_revolute(b1, b2, axis=(0, 0, 1), kinematic=newton.KinematicMode.POSITION) | ||
| template.add_articulation([j1, j2]) | ||
|
|
||
| num_worlds = 3 | ||
| builder = newton.ModelBuilder() | ||
| builder.replicate(template, num_worlds) | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model, disable_contacts=True) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
|
|
||
| # Set different position targets for each world | ||
| target = control.kinematic_target.numpy() | ||
| q_starts = model.joint_q_start.numpy() | ||
| joints_per_world = model.joint_count // num_worlds | ||
| target_angles = [0.1, 0.3, 0.5] | ||
| for w in range(num_worlds): | ||
| j2_idx = w * joints_per_world + 1 # second joint in each world | ||
| qs = q_starts[j2_idx] | ||
| target[qs] = target_angles[w] | ||
| control.kinematic_target = wp.array(target, dtype=wp.float32, device=model.device) | ||
|
|
||
| dt = 0.001 | ||
| for _ in range(100): | ||
| solver.step(state_0, state_1, control, None, dt) | ||
| state_0, state_1 = state_1, state_0 | ||
|
|
||
| # Check each world's kinematic hinge reached its target | ||
| joint_q = state_0.joint_q.numpy() | ||
| for w in range(num_worlds): | ||
| j2_idx = w * joints_per_world + 1 | ||
| qs = q_starts[j2_idx] | ||
| angle = float(joint_q[qs]) | ||
| self.assertAlmostEqual( | ||
| angle, | ||
| target_angles[w], | ||
| delta=0.05, | ||
| msg=f"World {w} hinge didn't reach target: {angle:.4f} vs {target_angles[w]}", | ||
| ) |
Contributor
There was a problem hiding this comment.
Update new test docstrings to Google-style and add SI units.
The new test docstrings are single-line summaries and omit [unit] annotations for physical quantities (e.g., velocity, position, dt). Please convert them to Google-style and include SI units such as [m], [rad], [rad/s], [m/s], [s] where relevant.
As per coding guidelines, "Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values" and "State SI units for all physical quantities in docstrings using inline [unit] notation (e.g., [m], [N], [N·m], or [m or rad, depending on joint type]); skip non-physical fields."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/tests/test_mujoco_solver.py` around lines 6085 - 6357, Several test
docstrings are one-line summaries and missing SI unit annotations; update the
docstrings for the listed test functions (e.g.,
test_kinematic_mode_builder_sets_armature,
test_kinematic_mode_none_no_armature_override,
test_kinematic_velocity_target_applied,
test_kinematic_position_target_computes_velocity,
test_kinematic_body_stays_in_place, test_kinematic_add_body_convenience,
test_kinematic_revolute_joint, test_kinematic_multiworld_velocity,
test_kinematic_multiworld_position) to Google-style: include a short summary
line, a blank line, and an optional Args/Returns section as appropriate (even
for tests), and annotate all physical quantities with SI units inline (e.g.,
[m], [rad], [rad/s], [m/s], [s]); ensure docstrings mention which variables have
units (dt, position/velocity/angle targets, z heights, etc.) and keep
descriptions concise and consistent across these test_* functions.
Comment on lines
6122
to
6155
| def test_kinematic_velocity_target_applied(self): | ||
| """Test that VELOCITY mode kinematic target is applied to joint_qd.""" | ||
| builder = newton.ModelBuilder() | ||
| body = builder.add_link( | ||
| xform=wp.transform(wp.vec3(0.0, 0.0, 1.0)), | ||
| mass=1.0, | ||
| inertia=wp.mat33(np.eye(3)), | ||
| ) | ||
| builder.add_shape_box(body, hx=0.1, hy=0.1, hz=0.1) | ||
| joint = builder.add_joint_free(body, kinematic=newton.KinematicMode.VELOCITY) | ||
| builder.add_articulation([joint]) | ||
| builder.add_ground_plane() | ||
| model = builder.finalize() | ||
|
|
||
| solver = SolverMuJoCo(model) | ||
| state_0 = model.state() | ||
| state_1 = model.state() | ||
| control = model.control() | ||
| newton.eval_fk(model, model.joint_q, model.joint_qd, state_0) | ||
|
|
||
| # Set angular velocity target: wz = 2.0 rad/s | ||
| q_start = int(model.joint_q_start.numpy()[joint]) | ||
| target = control.kinematic_target.numpy() | ||
| target[q_start + 5] = 2.0 # wz | ||
| control.kinematic_target = wp.array(target, dtype=wp.float32, device=model.device) | ||
|
|
||
| dt = 0.01 | ||
| solver.step(state_0, state_1, control, None, dt) | ||
|
|
||
| # Check that the output state has angular velocity close to 2.0 | ||
| qd = state_1.joint_qd.numpy() | ||
| qd_start = int(model.joint_qd_start.numpy()[joint]) | ||
| self.assertAlmostEqual(float(qd[qd_start + 5]), 2.0, delta=0.1) | ||
|
|
Contributor
There was a problem hiding this comment.
Use joint_qd_start for VELOCITY targets to avoid mis-indexing.
VELOCITY mode is per-DOF, so indexing with joint_q_start will drift when coord_dim ≠ dof_dim (free/ball) and can mis-target in multi-world replication. Use joint_qd_start in the VELOCITY tests to align with the API and avoid false positives.
🔧 Suggested fix
- q_start = int(model.joint_q_start.numpy()[joint])
+ qd_start = int(model.joint_qd_start.numpy()[joint])
target = control.kinematic_target.numpy()
- target[q_start + 5] = 2.0 # wz
+ target[qd_start + 5] = 2.0 # wz- q_starts = model.joint_q_start.numpy()
+ qd_starts = model.joint_qd_start.numpy()
for w in range(num_worlds):
joint_idx = w # one joint per world
- qs = q_starts[joint_idx]
- target[qs + 5] = float(w + 1) # wz = 1, 2, 3 rad/s
+ qd = qd_starts[joint_idx]
+ target[qd + 5] = float(w + 1) # wz = 1, 2, 3 rad/sAlso applies to: 6265-6297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@newton/tests/test_mujoco_solver.py` around lines 6122 - 6155, The test is
indexing the VELOCITY kinematic target using joint_q_start which can mis-index
DOF-specific velocity targets; update test_kinematic_velocity_target_applied to
use model.joint_qd_start (and the joint_qd_start variable) when computing
qd_start and when writing into control.kinematic_target so the target is placed
at the per-DOF velocity index (replace uses of joint_q_start → joint_qd_start
and ensure qd_start is derived from model.joint_qd_start.numpy()[joint]); apply
the same change to the other VELOCITY-mode tests referenced (around lines
6265-6297) so all velocity-target indexing uses joint_qd_start.
Member
|
First of all - amazing stuff. Nice that this works. Before I dive deeper - given that this is MuJoCo-specific, is it possible to set all of this up in the mujoco conversion? Otherwise this needs a deep API discussion around kinematics in all the other solvers. Especially relevant in context of the other draft PR Eric just pushed. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
newton/_src/solvers/mujoco/kernels.py (1)
1950-1953: VELOCITY mode for BALL/FREE joints: kinematic_target uses coordinate-space indexing (wq_i), so BALL angular velocity occupies slots 0–2 of the 4-slot quaternion region, and FREE angular velocity occupies slots 3–5 of the 7-slot coordinate region (slot 6 is unused in VELOCITY mode).This is a non-obvious convention. Consider adding a comment here (and/or in the public API docstring) clarifying:
- For BALL VELOCITY mode: write 3 angular-velocity components (in child body frame) at
kinematic_target[wq_i : wq_i+3].- For FREE VELOCITY mode: write 6 velocity components (linear world-frame + angular world-frame) at
kinematic_target[wq_i : wq_i+6]; slotwq_i+6is ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1950 - 1953, Add a clarifying comment (and update the public API docstring if present) next to the VELOCITY branch where mode == int(KinematicMode.VELOCITY) to explicitly document the coordinate-space indexing convention used by kinematic_target: for BALL joints the 3 child-frame angular-velocity components live at kinematic_target[wq_i : wq_i+3] (they occupy the first 3 slots of the 4-slot quaternion region), and for FREE joints the 6 velocity components (3 linear world-frame + 3 angular world-frame) live at kinematic_target[wq_i : wq_i+6] with kinematic_target[wq_i+6] unused in VELOCITY mode; mention that this is why the loop copies from kinematic_target[wq_i + i] into joint_qd[wqd_i + i].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1995-2018: The BALL joint path computes omega in the parent frame
(dq/tgt_q/cur_q are child-orientation-relative-to-parent), but MuJoCo expects
ball qvel in the child frame; after computing omega (the variable named omega)
rotate it into the child body frame by applying the inverse of cur_q (e.g. use
quat-rotation with wp.quat_inverse(cur_q)) before writing into joint_qd[wqd_i +
0..2]; update the block in kernels.py (the elif jtype == int(JointType.BALL)
path) so convert omega to child-frame (using
wp.quat_rotate(wp.quat_inverse(cur_q), omega) or equivalent) prior to assigning
joint_qd, and ensure convert_warp_coords_to_mj_kernel then passes the
correctly-framed qvel to MuJoCo.
---
Nitpick comments:
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1950-1953: Add a clarifying comment (and update the public API
docstring if present) next to the VELOCITY branch where mode ==
int(KinematicMode.VELOCITY) to explicitly document the coordinate-space indexing
convention used by kinematic_target: for BALL joints the 3 child-frame
angular-velocity components live at kinematic_target[wq_i : wq_i+3] (they occupy
the first 3 slots of the 4-slot quaternion region), and for FREE joints the 6
velocity components (3 linear world-frame + 3 angular world-frame) live at
kinematic_target[wq_i : wq_i+6] with kinematic_target[wq_i+6] unused in VELOCITY
mode; mention that this is why the loop copies from kinematic_target[wq_i + i]
into joint_qd[wqd_i + i].
- Fix world_count attribute (was num_worlds) in solver step - Fix BALL joint POSITION mode angular velocity frame conversion - Preserve joint_kinematic_mode through collapse_fixed_joints - Avoid mutating caller-supplied JointDofConfig armature - Add SI units to KinematicMode and kinematic_target docstrings - Add kinematic parameter docstring to add_body() - Remove private _paused access from example Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Member
Author
As I understand On the other hand, all the stuff in this PR is just helpers. Users can already do all the same with existing API: (1) set joint DOFs' armature to some big value, (2) write target velocity directly to joint_qd. So perhaps we can just leave it as is. At least we know now this approach works :) |
Member
|
Thanks - it's nice to have it though. Maybe we could make this an example somehow? I think users will expect automatic kinematic'ness if they follow eric's new API. But the target position/velocity stuff can be a documentation thing. |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/sim/builder.py (1)
70-75:⚠️ Potential issue | 🟡 MinorDocument the
kinematicparameter across all affected methods.
KinematicModeis properly re-exported in the public API. However, thekinematicparameter lacks docstring documentation in most methods. Add parameter descriptions toadd_joint(),add_joint_revolute(),add_joint_prismatic(),add_joint_ball(), andadd_joint_free()similar to the existing documentation inadd_body()(e.g., "Kinematic mode for the joint (see :class:KinematicMode). UseNONEfor dynamic;VELOCITYorPOSITIONdrives the joint viacontrol.kinematic_target.").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/builder.py` around lines 70 - 75, Add a docstring parameter description for the kinematic parameter in the methods add_joint, add_joint_revolute, add_joint_prismatic, add_joint_ball, and add_joint_free to match add_body's style: mention it is a KinematicMode (see :class:`KinematicMode`), and give the allowed values and behavior (e.g., "Kinematic mode for the joint (see :class:`KinematicMode`). Use NONE for dynamic; VELOCITY or POSITION drives the joint via control.kinematic_target."). Update the docstring parameter blocks in each of those functions (symbols: add_joint, add_joint_revolute, add_joint_prismatic, add_joint_ball, add_joint_free) so the kinematic parameter is consistently documented across the public API.
🧹 Nitpick comments (4)
newton/examples/basic/example_basic_kinematic_platform.py (3)
220-222:advance_timelaunch missing explicitdevice=argumentInconsistent with the
update_hinge_targetslaunch three lines above that explicitly passesdevice=self.model.device. Add it for consistency:♻️ Proposed fix
- wp.launch(advance_time, dim=1, inputs=[self.sim_time_wp, self.sim_dt]) + wp.launch(advance_time, dim=1, inputs=[self.sim_time_wp, self.sim_dt], device=self.model.device)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 220 - 222, The wp.launch call that starts advance_time should include the explicit device argument like the nearby update_hinge_targets launch; update the wp.launch invocation for advance_time (which currently uses dim=1, inputs=[self.sim_time_wp, self.sim_dt]) to pass device=self.model.device so it runs on the same device as the model and remains consistent with update_hinge_targets.
236-241:test_finalthresholdz > -0.5is excessively lenientA body 49 cm below ground still passes. Consider tightening to
z > -0.1(one capsule radius of slack) to catch genuine penetration failures.♻️ Proposed fix
- assert z > -0.5, f"Body {i} fell through ground: z={z:.4f}" + assert z > -0.1, f"Body {i} fell through ground: z={z:.4f}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 236 - 241, The assertion in test_final allowing bodies to be below ground by up to 0.5m is too lenient; update the threshold in test_final to require z > -0.1 (one capsule radius slack) when checking body positions from self.state_0.body_q.numpy() over range(self.model.body_count), and keep the existing error message format so failing bodies report their index and actual z value.
170-186: Redundant intermediatewp.arrayallocation at line 173The
wp.arrayassigned toself.control.kinematic_targetat line 173 is immediately discarded when line 186 re-creates it from the sametargetbuffer. Remove line 173 and batch all mutations before the single upload at line 186.♻️ Proposed fix
target = self.control.kinematic_target.numpy() target[platform_q_start + 5] = PLATFORM_ANGULAR_VEL -self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) # Collect hinge q_start indices for the kernel q_starts = self.model.joint_q_start.numpy() hinge_q_starts = [int(q_starts[j]) for j in self.hinge_joints] self.hinge_q_starts_wp = wp.array(hinge_q_starts, dtype=wp.int32, device=self.model.device) self.sim_time_wp = wp.zeros(1, dtype=wp.float32, device=self.model.device) # Initialize hinge position targets to match t=0 values (avoid jump on first step) for ci, qs in enumerate(hinge_q_starts): phase = ci * 0.5 target[qs] = HINGE_AMPLITUDE * math.sin(phase) self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 170 - 186, Remove the redundant intermediate upload of the numpy buffer to a wp.array: don't assign self.control.kinematic_target right after mutating target for PLATFORM_ANGULAR_VEL; instead keep mutating the local target array (used when setting platform angular velocity and when initializing hinge positions via hinge_q_starts) and perform a single wp.array(...) conversion/assignment to self.control.kinematic_target only once after all mutations (the final conversion currently done where the hinge loop finishes), ensuring you only call wp.array for target one time.newton/_src/solvers/mujoco/kernels.py (1)
1934-1938: Docstring missingArgssection and SI unit annotationsPer coding guidelines, docstrings should follow Google style and annotate physical quantities with SI units. The current docstring lacks an
Argsblock and doesn't specify thatinv_dtis[1/s]or thatkinematic_targetentries are[m/s or rad/s](VELOCITY) /[m or rad](POSITION).♻️ Proposed docstring expansion
- """Apply kinematic targets to joint velocities before MuJoCo step. - - For VELOCITY mode: copies target values directly to joint_qd. - For POSITION mode: computes velocity from (target - current) / dt. - """ + """Apply kinematic targets to joint velocities before MuJoCo step. + + For VELOCITY mode: copies target values directly to joint_qd. + For POSITION mode: computes velocity from (target - current) / dt. + + Args: + joint_kinematic_mode: Per-template-joint kinematic mode (KinematicMode enum). + joint_type: Per-template-joint type (JointType enum). + joint_q_start: Flattened per-world-joint coordinate start indices. + joint_qd_start: Flattened per-world-joint DOF start indices. + joint_dof_dim: Per-template-joint DOF dimension [n_lin, n_ang]. + kinematic_target: Kinematic target array indexed by joint_q_start offsets. + VELOCITY mode: per-DOF velocities [m/s or rad/s]. + POSITION mode: per-coordinate positions [m or rad]. + joint_q: Current joint coordinates (flat multi-world array). + inv_dt: Inverse of the timestep [1/s], used in POSITION mode. + joints_per_world: Number of joints per world. + joint_qd: Output joint DOF velocities (flat multi-world array). + """As per coding guidelines, "Use Google-style docstrings with clear, concise explanations of function behavior, parameters, and return values. State SI units for all physical quantities in docstrings using inline [unit] notation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/solvers/mujoco/kernels.py` around lines 1934 - 1938, Add a Google-style Args section to the function docstring that documents parameters including inv_dt and kinematic_target and annotate SI units: state inv_dt as "[1/s]", explain kinematic_target entries are either target velocities "[m/s]" or "[rad/s]" when in VELOCITY mode and target positions "[m]" or "[rad]" when in POSITION mode, and mention that joint_qd is set from these targets (or computed via (target - current)/dt); reference the relevant symbols in the docstring (inv_dt, kinematic_target, joint_qd, VELOCITY, POSITION) and keep the behavior description concise and consistent with existing Google-style formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@newton/_src/sim/builder.py`:
- Around line 70-75: Add a docstring parameter description for the kinematic
parameter in the methods add_joint, add_joint_revolute, add_joint_prismatic,
add_joint_ball, and add_joint_free to match add_body's style: mention it is a
KinematicMode (see :class:`KinematicMode`), and give the allowed values and
behavior (e.g., "Kinematic mode for the joint (see :class:`KinematicMode`). Use
NONE for dynamic; VELOCITY or POSITION drives the joint via
control.kinematic_target."). Update the docstring parameter blocks in each of
those functions (symbols: add_joint, add_joint_revolute, add_joint_prismatic,
add_joint_ball, add_joint_free) so the kinematic parameter is consistently
documented across the public API.
---
Duplicate comments:
In `@newton/_src/sim/builder.py`:
- Around line 2796-2817: The docstrings for the joint helper methods (e.g.,
add_joint, add_joint_revolute, add_joint_prismatic, add_joint_ball,
add_joint_free) are missing documentation for the new kinematic parameter;
update each method's Args section to include a concise Google-style description
for kinematic (type KinematicMode, default KinematicMode.NONE) that explains its
purpose (to mark joint kinematic behavior), accepted values, and effect on the
joint (e.g., treated as kinematic vs dynamic), mirroring the phrasing across the
add_joint variants and the other listed regions (around lines ~2796, ~2983-3051,
~3078-3145, ~3160-3220, ~3278-3320) so the docs remain consistent.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 66-190: Add a new row to the README.md "Basic Examples" table for
the new example_basic_kinematic_platform.py example: include a GitHub link to
newton/examples/basic/example_basic_kinematic_platform.py, a screenshot image
entry following the existing screenshot naming/pattern used by other examples,
and the run command "uv run -m newton.examples basic_kinematic_platform"
(matching the existing examples' command format). Ensure the table row follows
the exact column order and markdown formatting used by the other entries so the
table renders consistently; use the example file name and the Example class as
identifiers when verifying the screenshot and link.
---
Nitpick comments:
In `@newton/_src/solvers/mujoco/kernels.py`:
- Around line 1934-1938: Add a Google-style Args section to the function
docstring that documents parameters including inv_dt and kinematic_target and
annotate SI units: state inv_dt as "[1/s]", explain kinematic_target entries are
either target velocities "[m/s]" or "[rad/s]" when in VELOCITY mode and target
positions "[m]" or "[rad]" when in POSITION mode, and mention that joint_qd is
set from these targets (or computed via (target - current)/dt); reference the
relevant symbols in the docstring (inv_dt, kinematic_target, joint_qd, VELOCITY,
POSITION) and keep the behavior description concise and consistent with existing
Google-style formatting.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 220-222: The wp.launch call that starts advance_time should
include the explicit device argument like the nearby update_hinge_targets
launch; update the wp.launch invocation for advance_time (which currently uses
dim=1, inputs=[self.sim_time_wp, self.sim_dt]) to pass device=self.model.device
so it runs on the same device as the model and remains consistent with
update_hinge_targets.
- Around line 236-241: The assertion in test_final allowing bodies to be below
ground by up to 0.5m is too lenient; update the threshold in test_final to
require z > -0.1 (one capsule radius slack) when checking body positions from
self.state_0.body_q.numpy() over range(self.model.body_count), and keep the
existing error message format so failing bodies report their index and actual z
value.
- Around line 170-186: Remove the redundant intermediate upload of the numpy
buffer to a wp.array: don't assign self.control.kinematic_target right after
mutating target for PLATFORM_ANGULAR_VEL; instead keep mutating the local target
array (used when setting platform angular velocity and when initializing hinge
positions via hinge_q_starts) and perform a single wp.array(...)
conversion/assignment to self.control.kinematic_target only once after all
mutations (the final conversion currently done where the hinge loop finishes),
ensuring you only call wp.array for target one time.
eric-heiden
reviewed
Feb 18, 2026
newton/_src/sim/joints.py
Outdated
| return ActuatorMode.EFFORT | ||
|
|
||
|
|
||
| class KinematicMode(IntEnum): |
Member
There was a problem hiding this comment.
If we decide to have another enum for this it should end with "type" like the other ones, i.e. KinematicType.
Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
newton/_src/sim/model.py (1)
704-704:joint_kinematic_typefrequency entry is placed in the wrong comment section.It uses
Model.AttributeFrequency.JOINTbut sits inside the# attributes per joint dofblock (line 697). Move it to the# attributes per jointblock (afterjoint_twist_upper, line ~691) to keep the grouping consistent and avoid misleading readers.♻️ Proposed move
self.attribute_frequency["joint_twist_lower"] = Model.AttributeFrequency.JOINT self.attribute_frequency["joint_twist_upper"] = Model.AttributeFrequency.JOINT + self.attribute_frequency["joint_kinematic_type"] = Model.AttributeFrequency.JOINT # attributes per joint coord ... # attributes per joint dof self.attribute_frequency["joint_qd"] = Model.AttributeFrequency.JOINT_DOF ... self.attribute_frequency["joint_act_mode"] = Model.AttributeFrequency.JOINT_DOF - self.attribute_frequency["joint_kinematic_type"] = Model.AttributeFrequency.JOINT self.attribute_frequency["joint_target_ke"] = Model.AttributeFrequency.JOINT_DOF🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` at line 704, Move the assignment for self.attribute_frequency["joint_kinematic_type"] = Model.AttributeFrequency.JOINT out of the "# attributes per joint dof" block and place it into the "# attributes per joint" block (i.e., after the existing entry for joint_twist_upper). Locate the attribute_frequency population code and relocate the joint_kinematic_type line so it groups with other per-joint entries (keeping the exact key "joint_kinematic_type" and the value Model.AttributeFrequency.JOINT unchanged).newton/examples/basic/example_basic_kinematic_platform.py (1)
182-186:kinematic_targetfor POSITION-mode hinges is already initialized correctly bymodel.control()— re-initialization here is redundant.
Model.control()(seemodel.pylines 824–833) initializeskinematic_targetfromjoint_qfor all joints; POSITION-mode slots are left as-is. Sincebuilder.joint_q[hinge_q_start]was set toHINGE_AMPLITUDE * math.sin(phase)at line 152 beforefinalize(), those values are already present inself.control.kinematic_targetafter line 165. The loop at lines 183–186 writes the same values a second time. The explanatory comment is still useful, but the code could be simplified:♻️ Simplification
- # Initialize hinge position targets to match t=0 values (avoid jump on first step) - for ci, qs in enumerate(hinge_q_starts): - phase = ci * 0.5 - target[qs] = HINGE_AMPLITUDE * math.sin(phase) - self.control.kinematic_target = wp.array(target, dtype=wp.float32, device=self.model.device) + # NOTE: model.control() already initializes kinematic_target for POSITION-mode + # joints from joint_q, which was pre-seeded above (lines 151-152) to match the + # t=0 hinge targets — no first-step velocity spike.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/examples/basic/example_basic_kinematic_platform.py` around lines 182 - 186, The loop that re-writes POSITION-mode hinge targets is redundant because Model.control() already initializes self.control.kinematic_target from builder.joint_q (see Model.control()), so remove the for-loop that assigns target[qs] and the subsequent assignment to self.control.kinematic_target; instead keep the explanatory comment about avoiding a jump on the first step and rely on the existing initialization from builder.joint_q (hinge_q_starts, HINGE_AMPLITUDE, and finalize() are the relevant symbols to verify).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@newton/_src/sim/builder.py`:
- Around line 2796-2817: The docstrings for add_joint and its helpers
(add_joint_revolute, add_joint_prismatic, add_joint_ball, add_joint_free) were
not updated to document the new kinematic: KinematicType parameter; update each
function/class docstring to add a concise Google-style parameter entry for
"kinematic" that states its type (KinematicType), purpose (e.g., whether the
joint is kinematic vs dynamic and how it affects solver/constraints), default
value (KinematicType.NONE), and any important semantics or interactions (e.g.,
how it affects enabled/disabled behavior or DOF handling), and mirror the same
text into the helper signatures mentioned in the review so all joint APIs
consistently document kinematic.
In `@newton/_src/sim/model.py`:
- Line 822: Remove the now-unused noqa directive on the local import: delete the
trailing "# noqa: PLC0415" from the import statement that brings in
KinematicType (from .joints import KinematicType) since PLC0415 is not enabled;
leave the import as a plain local import or, if the import must be conditional
to avoid circular imports, replace the directive with an in-code comment
explaining the conditional/local import rationale (e.g., "local import to avoid
circular dependency") rather than an ineffective noqa tag.
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 66-190: The new example file example_basic_kinematic_platform.py
is not registered in the project docs and examples registry; add an entry to the
Basic Examples table in README.md including the file name, a short description,
a screenshot link, and the run command `uv run -m newton.examples
basic_kinematic_platform`, and also export/register the example in
newton/examples/__init__.py (same symbol/name used for other examples) so the
test command rg "basic_kinematic_platform" finds it; ensure the README row
format matches existing rows and that the module name matches the file
(basic_kinematic_platform).
In `@newton/tests/test_mujoco_solver.py`:
- Around line 6122-6154: In test_kinematic_velocity_target_applied the VELOCITY
target is being placed using joint_q_start (q_start) which can mis-index per-DOF
kinematic targets; change the indexing to use joint_qd_start (e.g., read
model.joint_qd_start for the joint index) when writing into
control.kinematic_target so the target is placed at the correct DOF position for
the joint (update the q_start usage to qd_start in this test and any analogous
tests like the ones around lines 6265-6296).
- Around line 6085-6357: Update each test method's one-line docstring (e.g.,
test_kinematic_type_builder_sets_armature,
test_kinematic_type_none_no_armature_override,
test_kinematic_velocity_target_applied,
test_kinematic_position_target_computes_velocity,
test_kinematic_body_stays_in_place, test_kinematic_add_body_convenience,
test_kinematic_revolute_joint, test_kinematic_multiworld_velocity,
test_kinematic_multiworld_position) to a Google-style multi-line docstring: a
short summary line, a blank line, then a longer description and any relevant
parameter/return notes; include SI units for physical quantities mentioned
(e.g., mass [kg], distance/position [m], velocity [m/s] or angular velocity
[rad/s], angle [rad], time step dt [s], forces/torques if any [N]/[N·m]) using
inline [unit] notation, and omit units for non-physical fields. Ensure each test
docstring is updated consistently to follow the repository docstring guidelines.
---
Nitpick comments:
In `@newton/_src/sim/model.py`:
- Line 704: Move the assignment for
self.attribute_frequency["joint_kinematic_type"] =
Model.AttributeFrequency.JOINT out of the "# attributes per joint dof" block and
place it into the "# attributes per joint" block (i.e., after the existing entry
for joint_twist_upper). Locate the attribute_frequency population code and
relocate the joint_kinematic_type line so it groups with other per-joint entries
(keeping the exact key "joint_kinematic_type" and the value
Model.AttributeFrequency.JOINT unchanged).
In `@newton/examples/basic/example_basic_kinematic_platform.py`:
- Around line 182-186: The loop that re-writes POSITION-mode hinge targets is
redundant because Model.control() already initializes
self.control.kinematic_target from builder.joint_q (see Model.control()), so
remove the for-loop that assigns target[qs] and the subsequent assignment to
self.control.kinematic_target; instead keep the explanatory comment about
avoiding a jump on the first step and rely on the existing initialization from
builder.joint_q (hinge_q_starts, HINGE_AMPLITUDE, and finalize() are the
relevant symbols to verify).
…t init Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
newton/_src/sim/model.py (1)
819-833:kinematic_targetis always allocated regardless ofclone_variables=False.All other control arrays respect
clone_variables=Falseby taking a reference instead of cloning. This block sits outside theif clone_variables/elsebranches and therefore always performs awp.arrayallocation and two.numpy()round-trips even when the caller passesclone_variables=Falseto suppress allocations. Since there is no pre-existing model-levelkinematic_targetto borrow a reference from, the allocation can't be avoided, but the asymmetry should at minimum be documented. A minimal improvement is to add a comment noting the exception, and if avoidance is desirable you could store a pre-computedkinematic_targeton the model at finalize time (like all other control arrays) so the normal clone/reference path applies here too.📝 Minimal fix: document the exception and avoid GPU round-trip when nothing needs zeroing
- # Initialize kinematic_target from joint_q, zeroing velocity-mode entries - if self.joint_count and self.joint_kinematic_type is not None: - from .joints import KinematicType # noqa: PLC0415 - - target_np = self.joint_q.numpy() - modes = self.joint_kinematic_type.numpy() - vel_joints = np.where(modes == int(KinematicType.VELOCITY))[0] - if len(vel_joints) > 0: - q_starts = self.joint_q_start.numpy() - dof_dims = self.joint_dof_dim.numpy() - for j in vel_joints: - qs = q_starts[j] - target_np[qs : qs + dof_dims[j, 0] + dof_dims[j, 1]] = 0.0 - c.kinematic_target = wp.array(target_np, dtype=wp.float32, device=self.device, requires_grad=requires_grad) + # Initialize kinematic_target from joint_q, zeroing velocity-mode entries. + # Note: unlike other control arrays this always allocates a new wp.array because + # no model-level kinematic_target exists to reference; clone_variables=False + # does not suppress this allocation. + if self.joint_count and self.joint_kinematic_type is not None: + from .joints import KinematicType + + modes = self.joint_kinematic_type.numpy() + vel_joints = np.where(modes == int(KinematicType.VELOCITY))[0] + if len(vel_joints) > 0: + target_np = self.joint_q.numpy() + q_starts = self.joint_q_start.numpy() + dof_dims = self.joint_dof_dim.numpy() + for j in vel_joints: + qs = q_starts[j] + target_np[qs : qs + dof_dims[j, 0] + dof_dims[j, 1]] = 0.0 + c.kinematic_target = wp.array(target_np, dtype=wp.float32, device=self.device, requires_grad=requires_grad) + else: + c.kinematic_target = wp.clone(self.joint_q, requires_grad=requires_grad)This avoids the unconditional
.numpy()round-trip onjoint_qwhen no VELOCITY joints exist. As per coding guidelines, the# noqa: PLC0415should also be dropped from the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@newton/_src/sim/model.py` around lines 819 - 833, The kinematic_target allocation always happens regardless of clone_variables and performs unnecessary .numpy() round-trips; update the block around kinematic_target to (1) add a clear comment stating kinematic_target is an exception to the clone_variables/reference behavior and why it must be allocated if not precomputed, (2) avoid GPU-to-CPU round-trips when no velocity joints exist by checking joint_kinematic_type / vel_joints first and only calling joint_q.numpy(), joint_q_start.numpy(), and joint_dof_dim.numpy() when len(vel_joints) > 0, and (3) remove the unnecessary "# noqa: PLC0415" on the KinematicType import; keep references to symbols kinematic_target, clone_variables, joint_count, joint_kinematic_type, KinematicType, joint_q, joint_q_start, and joint_dof_dim to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@newton/_src/sim/model.py`:
- Line 822: The import line "from .joints import KinematicType # noqa: PLC0415"
in module newton._src.sim.model contains an unused noqa directive; remove the
trailing " # noqa: PLC0415" so the line is simply "from .joints import
KinematicType" (ensure the import is still used or kept intentionally), and run
the linter to confirm the PLC0415 suppression is no longer present.
---
Nitpick comments:
In `@newton/_src/sim/model.py`:
- Around line 819-833: The kinematic_target allocation always happens regardless
of clone_variables and performs unnecessary .numpy() round-trips; update the
block around kinematic_target to (1) add a clear comment stating
kinematic_target is an exception to the clone_variables/reference behavior and
why it must be allocated if not precomputed, (2) avoid GPU-to-CPU round-trips
when no velocity joints exist by checking joint_kinematic_type / vel_joints
first and only calling joint_q.numpy(), joint_q_start.numpy(), and
joint_dof_dim.numpy() when len(vel_joints) > 0, and (3) remove the unnecessary
"# noqa: PLC0415" on the KinematicType import; keep references to symbols
kinematic_target, clone_variables, joint_count, joint_kinematic_type,
KinematicType, joint_q, joint_q_start, and joint_dof_dim to locate the code.
…upport-kinematic-bodies Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com> # Conflicts: # newton/_src/solvers/mujoco/solver_mujoco.py
Signed-off-by: Viktor Reutskyy <vreutskyy@nvidia.com>
eric-heiden
reviewed
Feb 19, 2026
|
|
||
| # Armature override for kinematic joints | ||
| KINEMATIC_ARMATURE = 1.0e10 | ||
| armature_override = KINEMATIC_ARMATURE if kinematic != KinematicType.NONE else None |
Member
There was a problem hiding this comment.
Should we do something about child bodies that are connected by kinematic joints, for example enforce zero mass on them? That would play nicely with what I am doing in #1657: links must explicitly declare mass=0 and only then can they be connected by kinematic joints. Otherwise maximal coordinate solvers (XPBD, SemiImplicit) will still integrate all rigid bodies with nonzero mass and it will be difficult to enforce kinematic joint constraints this way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #1115
Adds first-class kinematic joint support for
SolverMuJoCo. Instead of using MuJoCo's mocap bodies (which teleport without velocity and break contact resolution), this approach emulates kinematic behavior by setting very high armature (1e10) on joint DOFs. The solver sees proper velocity, so contacts, friction, and impulses all work correctly.Approach
Kinematic is a joint property, not a body property. Any joint can be made kinematic — a free joint (for kinematic bodies), a revolute joint (for position/velocity-controlled hinges), etc. The high armature means
1/armature ≈ 0in the inverse mass matrix, so external forces can't accelerate the DOF, but it still participates in velocity-level constraint solving.New API
KinematicTypeenum (NONE,VELOCITY,POSITION):control.kinematic_target— per-coordinate target array (same size/indexing asjoint_q):VELOCITYmode: write DOF velocities atq_startoffsetsPOSITIONmode: write coordinate positions atq_startoffsets (solver computes velocity internally as(target - current) / dt)What's included
KinematicModeenum innewton/_src/sim/joints.pykinematicparameter onadd_joint(),add_joint_free(),add_joint_revolute(),add_joint_prismatic(),add_joint_ball(),add_body()control.kinematic_targetarray with proper initializationapply_kinematic_target_kernelin MuJoCo kernels (GPU, graph-capture compatible)SolverMuJoCo.step()before state sync2026-02-18.11-31-44.mp4
Limitations
joint_qto match the initial target to avoid velocity spikes on the first stepKinematicModeNewton Migration Guide
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
New Features
Controls
Documentation
Examples
Tests