Fix non-finite depth display normalization - #7119
Conversation
Greptile SummaryFixes depth-image display normalization by replacing non-finite values with zero before calculating the finite display scale.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or integration issues identified. The implementation sanitizes non-finite values without changing tensor shape or device, computes normalization from finite depths, and handles entirely non-finite input without division. Important Files Changed
Reviews (1): Last reviewed commit: "docs: note depth display normalization f..." | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
The patch narrowly updates depth display normalization to replace non-finite values with zero before selecting the display scale, with focused regression coverage and a package changelog fragment.
- Design and architecture: The change remains within the display-normalization utility and preserves the existing convention that invalid or no-hit depth pixels display as black. It does not shift responsibility to camera producers or downstream callers.
- API: The function signature, output shape, float32 conversion, and device placement are unchanged. Non-finite depth values now deterministically produce zero-valued display pixels. The added torch.where also ensures a fresh depth output tensor, but aliasing is not documented as part of the API contract.
- Implementation: The revised depth path removes NaN and positive or negative infinity before computing the maximum, preserving contrast among finite positive depths. The existing max-value guard correctly leaves all-non-finite input as zeros. Tests cover mixed finite/non-finite values for all three supported depth display types and the all-non-finite boundary case; the required source-package changelog fragment is present.
No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.
Automated review; human maintainers own approval decisions.
8e0e178 to
9345e78
Compare
Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
9345e78 to
167b8aa
Compare
AntoineRichard
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR, and contributing to IsaacLab! @kellyguo11 @hujc7 can I have another review?
# Description `normalize_camera_output_for_display()` normalized depth images using the raw tensor maximum. Depth camera outputs may contain `inf` for no-hit pixels and can contain other non-finite values, so the maximum became non-finite; dividing by it could produce `NaN` pixels and suppress finite depth contrast. This change zeroes non-finite depth values before computing the display scale, preserving finite depth normalization while keeping no-hit pixels black. ## Validation Adds unit coverage for: - mixed finite, `inf`, and `NaN` depth values across supported depth display types; - all-non-finite depth input. ## Type of change - Bug fix --------- Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Co-authored-by: Antoine RICHARD <antoiner@nvidia.com> (cherry picked from commit 0a05bbd)
…7299) ## Summary Backports the following merged changes to `release/3.0.0` as separate provenance-preserving cherry-picks: - #7292 — scope Newton global imports with clone plans - #7285 — stabilize the sensor/PhysX video recording test - #7269 — streamline Newton contact and raycast sensor startup - #7119 — normalize non-finite depth display values safely - #7295 — avoid repeated Newton model and articulation startup work Each source squash commit was cherry-picked with `-x` and applied without conflicts. ## Validation - Stable patch IDs match all five source squash commits exactly. - File-by-file manifests match each source squash commit. - `git diff --check upstream/release/3.0.0..HEAD` - `uv run --frozen python tools/changelog/cli.py check backport-7285-7292-base` - `SKIP=check-changelog-fragments uv run --frozen isaaclab -f` - Cloner/Newton focused tests: 98 passed - Scene global-ownership tests: 2 passed - Simulator clone-plan tests: 4 passed - Video recording regression test: 1 passed - Newton BVH lifecycle tests: 2 passed - Newton contact-selector tests: 7 passed - Newton raycast BVH test: 4 passed - Non-finite depth display tests: 4 passed - #7295 physics lifecycle, cloner, manager, and coupling tests: 248 passed - #7295 Newton joint-wrench sensor tests: 11 passed - #7295 PhysX joint-wrench sensor tests: 16 passed PR #7121 remains open and is intentionally excluded; it will be backported from its final merge commit after merging. --------- Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Co-authored-by: matthewtrepte <mtrepte@nvidia.com> Co-authored-by: camevor <camevor@nvidia.com> Co-authored-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Co-authored-by: Antoine RICHARD <antoiner@nvidia.com>
Description
normalize_camera_output_for_display()normalized depth images using the raw tensor maximum. Depth camera outputs may containinffor no-hit pixels and can contain other non-finite values, so the maximum became non-finite; dividing by it could produceNaNpixels and suppress finite depth contrast.This change zeroes non-finite depth values before computing the display scale, preserving finite depth normalization while keeping no-hit pixels black.
Validation
Adds unit coverage for:
inf, andNaNdepth values across supported depth display types;Type of change