fix: Cache the resolved Warp device on the OVRTX renderer - #7169
Draft
pv-nvidia wants to merge 5 commits into
Draft
fix: Cache the resolved Warp device on the OVRTX renderer#7169pv-nvidia wants to merge 5 commits into
pv-nvidia wants to merge 5 commits into
Conversation
Add map_attribute_for_warp_writes, a reusable context manager that maps an OVRTX attribute binding for CUDA writes and unmaps it with the producing Warp stream as the CUDA sync, so OVRTX's commit of the mapped data waits for the fill on the GPU. The binding's own context manager cannot carry that sync: its __exit__ takes no arguments and unmaps with an empty cuda_sync, which the OVRTX API treats as no synchronization. Route the renderer's object and camera transform writes, the only GPU attribute mappings in the codebase, through the helper. Previously their ordering held only through CUDA legacy default-stream serialization against Warp's blocking-type streams, an implementation detail the OVRTX contract does not promise. Render-var CUDA maps already order their reads via _map_render_var_to_dlpack, and render-var CPU maps are synchronized by OVRTX before map() returns, so neither needs changes.
wp.get_device resolves the device ident once, so the mapped CUDA device and the sync stream come from the same device: a bare "cuda" previously parsed to index 0 while the stream lookup used Warp's current CUDA device, silently voiding the ordering guarantee on non-zero devices. The string parser is gone; the resolved device's ordinal and current stream are used for map and unmap. The ovrtx import moves inside the helper so importing the module through the package's lazy exports cannot initialize ovrtx without the guarded environment ovrtx_renderer establishes (OVRTX_SKIP_USD_CHECK, actionable install error). Any real binding is created through that path, so ovrtx is already imported when the helper runs.
The device-index resolution is one testable contract: it delegates to Warp so a bare "cuda" means Warp's current CUDA device, never a parse of the string. Pin that in a focused unit test and drop the map-level test that duplicated it; the map tests keep asserting only what the context manager owns (device forwarding, producer-stream unmap, and release on a failed fill).
…ollow-up Resolving the mapping device through Warp changed behavior for bare "cuda" strings (current CUDA device instead of index 0). That change is worth making, but not inside the stream-ordering fix: restore the string parse so this PR alters no device placement, and mark the bare-cuda / current-device split with a TODO. The follow-up caches the resolved Warp device on the renderer instead of re-deriving it from strings.
create_render_data resolves the Warp device once and normalizes the renderer's device string from it, so a bare "cuda" pins to Warp's current CUDA device instead of being re-interpreted per call site -- previously attribute mappings parsed it to device 0 while Warp enqueued the fill kernels and resolved sync streams on its current device, splitting the work across GPUs on multi-GPU processes. Every mapping device id and CUDA sync stream (transform mappings, particle ASYNC writes, render-var reads) now derives from the cached device, and map_attribute_for_warp_writes takes the resolved device instead of a string. The device's current stream is still read at use time, not cached: the stream can legitimately change (e.g. CUDA graph capture), the device cannot.
github-actions
Bot
added
documentation
7 tasks
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
1 participant
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
The OVRTX renderer re-derived its CUDA device per call site from the device string, which split a bare
"cuda"across GPUs on multi-GPU processes: attribute mappings parsed it to device 0 while Warp enqueued the fill kernels and resolved sync streams on its current CUDA device.The renderer now resolves the Warp device once in
create_render_data, normalizes its device string from it, and derives every mapping device id and CUDA sync stream (transform mappings, particle ASYNC writes, render-var reads) from the cached device.map_attribute_for_warp_writestakes the resolved Warp device instead of a string, so the mapped device and the sync stream cannot diverge at a call site. The device's current stream is still read at use time rather than cached — the stream legitimately changes (e.g. CUDA graph capture), the device does not.Stacked on #7166.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there