Newton raycaster + bvh refactor#6502
Conversation
github-actions
Bot
added
documentation
Greptile SummaryThis PR refactors the Newton ray-cast sensor stack, replacing the single monolithic
Confidence Score: 4/5The change is safe to merge; the new sensor graph scheduling and BVH refit batching are well-tested and the refactor correctly handles graph invalidation on task registration/removal. The core Newton manager sensor scheduling logic is sound — invalidation, fallback to eager, and the conditional CUDA graph via wp.capture_if are all handled consistently. The main concerns are non-blocking: a shared mutable config object in VelocityEnvHeightScannerCfg, a broad except RuntimeError in site registration that could mask real configuration mistakes, and PPISP running immediately after an async graph launch with an implicit same-stream ordering dependency. Tests cover eager and CUDA-graph paths as well as moving-geometry BVH refit. newton_raycast_sensor.py (_register_sites_for_expr error handling), newton_warp_renderer.py (PPISP ordering after graph launch), and velocity_env_cfg.py (shared config object aliases) Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ray_alignment="yaw", | ||
| pattern_cfg=patterns.GridPatternCfg(resolution=0.1, size=[1.6, 1.0]), | ||
| debug_vis=False, | ||
| global_world_only=True, |
There was a problem hiding this comment.
newton_kamino = newton_mjwarp and physx = default / ovphysx = default assign the same object reference to multiple class attributes. Any downstream code that mutates one variant (e.g. scene_cfg.height_scanner.newton_kamino.debug_vis = True) will silently modify the other variant pointing to the same instance, since Python does not copy the object. The same applies to physx and ovphysx sharing the default RayCasterCfg. Consider constructing a separate instance for each alias that is intended to be independently configurable (or clearly document that these variants are intentionally frozen/immutable).
| ray_directions=self._ray_directions_w_flat, | ||
| ray_worlds=self._ray_worlds, | ||
| enable_global_world=not bool(getattr(self.cfg, "global_world_only", False)), | ||
| out_dist=self._hit_dist, | ||
| out_normal=self._hit_normal, | ||
| ) | ||
| wp.launch( | ||
| _resolve_bvh_hits_kernel, | ||
| dim=(self._num_envs, self.num_rays), | ||
| inputs=[ | ||
| self._is_outdated, | ||
| self._ray_starts_w, | ||
| self._ray_directions_w, | ||
| self._hit_dist, | ||
| self._hit_normal, | ||
| float(self.cfg.max_distance), |
There was a problem hiding this comment.
except RuntimeError may swallow real configuration errors
The except RuntimeError block is meant to handle the case where _resolve_rigid_body_ancestor_expr() fails because the USD prim doesn't exist yet. However, it catches every RuntimeError from that call, including errors that indicate genuine misconfiguration (e.g., a body lookup failure caused by a malformed prim path or an unsupported prim type). In those cases the fallback silently uses prim_expr as the body expression, which can produce a Newton site attached to the wrong ancestor, leading to incorrect sensor poses at runtime with no warning. Consider catching a narrower exception type, or at minimum logging the swallowed error so it is visible during debugging.
| def render(self, render_data: RenderData): | ||
| """Render and write to output buffers. See :meth:`~isaaclab.renderers.base_renderer.BaseRenderer.render`.""" | ||
|
|
||
| newton_state: newton.State = NewtonManager.get_state() | ||
| # Refresh the shadow state under PhysX before the manager refits the BVH. | ||
| NewtonManager.get_state() | ||
| if render_data.sensor_task_name is None: | ||
| render_data.sensor_task_name = f"newton_warp_render:{id(render_data)}" | ||
| NewtonManager._register_sensor_task(render_data.sensor_task_name, lambda: self._launch_render(render_data)) | ||
| NewtonManager._update_sensor_tasks(render_data.sensor_task_name) | ||
|
|
||
| # Refit the shape BVH against the current state since env body poses move every frame. | ||
| # ``build_bvh_shape`` ran once in ``__init__``; ``refit_bvh_shape`` reuses that topology. | ||
| if self.newton_sensor.model.shape_count > 0: | ||
| newton.geometry.refit_bvh_shape(self.newton_sensor.model, newton_state) | ||
| # Post-render PPISP: HDR scene-linear → LDR RGBA. Source/destination | ||
| # tensors were bound once in ``set_outputs``. | ||
| if render_data.ppisp_pipeline is not None: | ||
| render_data.ppisp_pipeline.apply( | ||
| render_data._ppisp_hdr_source, | ||
| render_data._ppisp_rgba_dest, | ||
| ) | ||
|
|
||
| def _launch_render(self, render_data: RenderData) -> None: | ||
| """Launch the tiled-camera render kernels for sensor graph capture.""" |
There was a problem hiding this comment.
NewtonManager._update_sensor_tasks in CUDA-graph mode calls wp.capture_launch, which is asynchronous on the host: the render graph is queued on the warp stream but the host continues immediately. The PPISP apply() call on the lines that follow consumes the rendered HDR buffer. If ppisp_pipeline.apply() is backed exclusively by warp/CUDA operations on the same stream, ordering is guaranteed by the stream. If it dispatches to a different stream or to synchronous CPU code that then reads GPU memory without an explicit wp.synchronize_stream, the apply may begin before the render graph has produced its output. The old code called newton_sensor.update() eagerly before PPISP, so this ordering was implicit. Consider adding a comment that makes the stream dependency explicit, or inserting a stream synchronization point before PPISP when the graph path is active.
| debug_vis=False, | ||
| global_world_only=True, | ||
| ) | ||
| newton_kamino = newton_mjwarp |
There was a problem hiding this comment.
I see, I agree it looks a bit ugly, I guess its fine for now we can think of better solution
The Newton BVH ray caster PR repoints the Newton backend of RayCasterCamera, MultiMeshRayCaster, and MultiMeshRayCasterCamera to their Legacy warp-mesh classes, so the isaaclab package needs its own changelog fragment. Add a patch-level Changed entry; the change is transparent to users of the backend-dispatching sensors.
2 participants
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there