Bug Fixes for Mimic-Cosmos Workflows - #7500
Open
shauryadNv wants to merge 10 commits into
Open
Conversation
shauryadNv
requested review from
AntoineRichard,
ClemensSchwarke,
StafaH,
aserifi,
david-cao-mueller,
fatimaanes,
frlai,
hougantc-nvda,
huidongc,
hujc7,
kellyguo11,
marcodiiga,
matthewtrepte,
myurasov-nv,
ooctipus,
pbarejko,
peterd-NV,
r-schmitt,
rilei-nvidia,
rubengrandia and
rwiltz
as code owners
September 2, 2026 20:59
Contributor
|
Too many files changed for review (802 files, 100 file limit). |
github-actions
Bot
added
bug
shauryadNv
force-pushed
the
shauryad/bug_fixes
branch
from
September 2, 2026 21:01
36549c3 to
5d6e1a1
Compare
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Isaac Lab Review Bot
The warm-up buffer guard, explicit camera enablement in robomimic scripts, documentation updates, and changelog integration are consistent with their affected paths. The tiled-buffer preflight check needs refinement because its unconditional four-channel assumption rejects valid single-channel configurations that would remain within Warp's actual per-annotator flattening limit.
- Design and architecture: Fail-fast validation in create_render_data() is appropriately placed, but the validator models a universal worst-case buffer rather than the per-annotator buffers that render() actually flattens. Validation should reflect the requested data types and colorization behavior.
- API: The launcher changes preserve the existing script interfaces, and no public symbol is removed or renamed. However, the new create_render_data() precondition unnecessarily narrows the accepted camera configuration space for single-channel render outputs.
- Implementation: The channel-trimming restructuring preserves the prior trim widths and safely avoids slicing empty warm-up buffers. The overflow calculation mirrors the tile-grid layout, but its fixed four-elements-per-pixel multiplier must be replaced with a per-requested-annotator channel count, including segmentation colorization settings.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
huidongc
reviewed
Sep 3, 2026
huidongc
reviewed
Sep 3, 2026
shauryadNv
added a commit
to shauryadNv/IsaacLab
that referenced
this pull request
Sep 3, 2026
Addresses review feedback on isaac-sim#7500. render() flattened each annotator's tiled buffer into a single 1D Warp array, whose length is num_envs * tile pixels * channels. That product can exceed the maximum size of a single Warp array dimension, so large environment counts / camera resolutions failed with: ValueError: Array shapes must not exceed the maximum representable value of a signed 32-bit integer, got 2621440000 in dimension 0 Refactor reshape_tiled_image to index the tiled buffer as a 3D array (num_tiles_y * image_height, num_tiles_x * image_width, num_channels) and update the overloads to ndim=3, so each dimension is bounded individually rather than by their product. Single-channel annotators return a 2D buffer, so the renderer adds the trailing channel dimension before launching. This removes the need for the preflight size check added earlier in this PR, which is dropped along with the hardcoded Warp dimension limit: per review, that limit is a Warp implementation detail rather than a contract, and the check also assumed 4 elements per pixel for every camera, wrongly rejecting valid single-channel configurations. Verified against Warp 1.9.1 on GPU: the refactored kernel is numerically identical to the previous implementation across RGBA, depth, normals and ragged-grid cases; a (25600, 25600, 4) buffer -- the exact geometry from the reported failure -- still raises on .flatten() but launches successfully through the 3D path.
Contributor
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
IsaacRtxRenderer.render() unconditionally sliced the tiled data buffer down to 2/3 channels for motion_vectors, normals, SIMPLE_SHADING_MODES, and RGB_HDR outputs. Right after an annotator is attached (e.g. at env creation, before the RTX renderer has pumped a frame), Replicator can momentarily return a buffer whose channel dimension is 0 instead of real image data. Warp's array slicing rejects trimming an already-empty dimension (unlike NumPy, which allows it), so this raised RuntimeError: Invalid indexing in slice: 0:0:1 e.g. on Isaac-Contrib-Stack-Cube-Franka-IK-Rel-Visuomotor-Cosmos with --visualizer newton_gl. Guard the trim against the buffer not being populated yet and skip writing that data type for the frame instead of crashing; the next render() call picks up valid data.
release/3.0.0 removed the --enable_cameras CLI flag and the ENABLE_CAMERAS env-var fallback from AppLauncher, replacing them with launch_simulation()'s scene-scan auto-detection, which these legacy scripts never call. As a result, robust_eval.py and play.py launch without RTX rendering enabled for any camera-observation task (e.g. IsaacContrib-Stack-Cube-Franka-IK-Rel-Visuomotor-Cosmos), and since IsaacRtxRenderer's old explicit "pass --enable_cameras" RuntimeError guard was also removed in this release, the failure now surfaces as an opaque ValueError: Invalid object in Py_Graph in getWrappedGraphFromNode deep inside OmniGraph/SyntheticData instead. Pass enable_cameras=True explicitly, matching the pattern already used in generate_dataset.py, since both scripts always need camera observations to run rollouts.
The Augmented Imitation Learning doc otherwise recommends uv as the primary workflow (data generation, training, eval all show a "uv (Recommended)" tab), but the hdf5_to_mp4.py, mp4_to_hdf5.py, and merge_hdf5_datasets.py examples only showed a bare `python ...` invocation with no indication of which environment/extras to use. Add matching uv (Recommended) / isaaclab.sh tab-sets for these three conversion steps. These tools only need h5py/opencv/numpy (no Isaac Sim import), so use `--extra mimic` -- the same extra already used a few sections down to verify the robomimic install.
IsaacRtxRenderer.render() flattens every environment's camera tile into one Warp array per data type. Warp requires every array dimension to fit in a signed 32-bit int, so a large enough num_envs * resolution combination (e.g. TC_146493: IsaacContrib-Stack-Cube-Franka-IK-Rel-Visuomotor-Cosmos at scale) overflows that limit and crashes deep inside render()'s .flatten() call with an opaque ValueError: Array shapes must not exceed the maximum representable value of a signed 32-bit integer, got 2621440000 in dimension 0 with no indication of what to change. Compute the worst-case tiled buffer size upfront in create_render_data() -- before any USD/Replicator work happens -- and raise a clear ValueError naming the offending env count/grid/ resolution and suggesting to reduce --num_envs or camera resolution, instead of letting the crash surface from inside Warp. This does not lift the underlying Warp/RTX limit (a single tiled render product still can't back more elements than a signed int32 can index); it only replaces the opaque failure with an actionable one at the point the request is made.
- Extract the tiled-buffer size guard into _validate_tiled_buffer_size() to bring create_render_data()'s cyclomatic complexity back under ruff's C901 threshold (31 > 30). - Wrap the trim_channels condition line ruff-format flagged as too long. - Add the changelog.d fragment required for the touched isaaclab_physx package (scripts/ and docs/ changes in prior commits aren't gated by the changelog check).
Addresses review feedback on isaac-sim#7500. render() flattened each annotator's tiled buffer into a single 1D Warp array, whose length is num_envs * tile pixels * channels. That product can exceed the maximum size of a single Warp array dimension, so large environment counts / camera resolutions failed with: ValueError: Array shapes must not exceed the maximum representable value of a signed 32-bit integer, got 2621440000 in dimension 0 Refactor reshape_tiled_image to index the tiled buffer as a 3D array (num_tiles_y * image_height, num_tiles_x * image_width, num_channels) and update the overloads to ndim=3, so each dimension is bounded individually rather than by their product. Single-channel annotators return a 2D buffer, so the renderer adds the trailing channel dimension before launching. This removes the need for the preflight size check added earlier in this PR, which is dropped along with the hardcoded Warp dimension limit: per review, that limit is a Warp implementation detail rather than a contract, and the check also assumed 4 elements per pixel for every camera, wrongly rejecting valid single-channel configurations. Verified against Warp 1.9.1 on GPU: the refactored kernel is numerically identical to the previous implementation across RGBA, depth, normals and ragged-grid cases; a (25600, 25600, 4) buffer -- the exact geometry from the reported failure -- still raises on .flatten() but launches successfully through the 3D path.
shauryadNv
force-pushed
the
shauryad/bug_fixes
branch
from
September 3, 2026 22:19
c00a09b to
50d93ff
Compare
Contributor
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
CI (rendering-correctness, test_rendering_cartpole) caught semantic_segmentation, instance_segmentation and instance_id_segmentation_fast rendering as all-zero pixels after the switch to 3D indexing. Those three data types are the ones reinterpreted via wp.array(ptr=..., shape=(*shape, 4)) for colorization. When the raw annotator buffer is flat, that yields a 2D (pixels, 4) array rather than (rows, cols, channels), so inferring the layout from ndim and appending a trailing 1 produced (pixels, 4, 1). The kernel then read channel i in range(4) from a size-1 dimension, leaving the output buffer empty. The previous flatten() was insensitive to the incoming shape, which is why this only appeared after the refactor. Reshape the buffer explicitly to the geometry the kernel documents -- (num_tiles_y * height, num_tiles_x * width, channels), taking the channel count from the destination buffer -- instead of inferring it from ndim. This normalizes 1D, 2D and 3D annotator buffers uniformly. Verified against Warp on all annotator buffer shapes: 3D rgb/rgba, 2D (H, W) depth, 2D (pixels, 4) colorized segmentation, 1D non-colorized segmentation, trimmed 3-channel normals, and a ragged tile grid. The previous heuristic reproduces the wrong output for the (pixels, 4) case; the new code matches the reference for all of them.
Contributor
|
/isaaclab-review |
Contributor
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
kellyguo11
reviewed
Sep 4, 2026
robust_eval.py / play.py: rendering is not always required -- both scripts also evaluate policies trained on low-dimensional observations, which should not pay for the RTX renderer. Instead of hardcoding enable_cameras=True, resolve the task config before launching Kit (resolve_task_config is documented as safe pre-launch) and derive the flag from scan(...).has_kit_camera, the same detection launch_simulation uses to auto-enable cameras. As a side effect, an invalid/missing --task now fails before Kit starts rather than after. augmented_imitation.rst: recent Isaac Sim releases ship an OpenCV build without ffmpeg, which hdf5_to_mp4.py and mp4_to_hdf5.py need for MP4 read/write. Add a note under Cosmos Augmentation telling users to install the full OpenCV package first.
Contributor
Author
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
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
4 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
This PR bundles four fixes for issues found:
IsaacRtxRenderer.render()unconditionally trimmed the tiled buffer to 2/3 channels formotion_vectors/normals/SIMPLE_SHADING_MODES/RGB_HDR. Immediately after an annotator is attached (e.g. at env creation, before the RTX renderer has pumped a frame), Replicator can momentarily return a buffer whose channel dimension is 0. Warp's array slicing rejects trimming an already-empty dimension (unlike NumPy, which allows it), raisingRuntimeError: Invalid indexing in slice: 0:0:1. Guard the trim and skip writing that data type for the frame instead of crashing.robust_eval.py/play.py: passenable_cameras=Trueexplicitly.release/3.0.0removed the--enable_camerasCLI flag and theENABLE_CAMERASenv-var fallback fromAppLauncherin favor oflaunch_simulation()'s scene-scan auto-detection, but these two robomimic scripts still use the legacyAppLauncher(args_cli)pattern and were never migrated. Without cameras enabled, and with the old explicit "pass --enable_cameras" guard inIsaacRtxRenderer.__init__also removed in this release, the failure now surfaces as an opaqueValueError: Invalid object in Py_Graph in getWrappedGraphFromNodedeep inside OmniGraph/SyntheticData. Passenable_cameras=Trueexplicitly, matching the pattern already used ingenerate_dataset.py.uv runexamples for the HDF5/MP4 conversion and merge tools. The Augmented Imitation Learning doc recommendsuvas the primary workflow throughout (dataset generation, training, eval all show a "uv (Recommended)" tab), but thehdf5_to_mp4.py,mp4_to_hdf5.py, andmerge_hdf5_datasets.pyexamples only showed a barepython ...invocation with no indication of which environment/extras to use. Added matchinguv (Recommended)/isaaclab.shtab-sets (--extra mimic, since these tools only needh5py/opencv/numpy, no Isaac Sim import).IsaacRtxRenderer.render()flattens every environment's camera tile into one Warp array per data type. Warp requires every array dimension to fit in a signed 32-bit int, so a large enoughnum_envs * resolutioncombination overflows that limit and crashes deep insiderender()'s.flatten()call withValueError: Array shapes must not exceed the maximum representable value of a signed 32-bit integer, got 2621440000 in dimension 0and no indication of what to change. Compute the worst-case tiled buffer size upfront increate_render_data()and raise a clear error naming the offending env count/grid/resolution and suggesting to reduce--num_envsor camera resolution. This does not lift the underlying Warp/RTX limit — it only replaces the opaque failure with an actionable one.Type of change
Release backport
developChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there