Skip to content

fix raycaster visulization errors printed in the terminal#4519

Open
DreaverZhao wants to merge 2 commits intoisaac-sim:developfrom
DreaverZhao:raycaster_vis_fix
Open

fix raycaster visulization errors printed in the terminal#4519
DreaverZhao wants to merge 2 commits intoisaac-sim:developfrom
DreaverZhao:raycaster_vis_fix

Conversation

@DreaverZhao
Copy link
Copy Markdown
Contributor

Description

Fixes #4518 by not calling the visualize function if all ray_hits_w are invalid values. I've also re-sorted the contributors' names alphabetically.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Feb 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 3, 2026

Greptile Overview

Greptile Summary

This PR fixes a bug where raycaster visualization would throw errors when all ray hits are invalid (inf values). The fix adds an early return check after filtering infinite values to prevent calling the visualizer with an empty tensor, which would cause a ValueError in VisualizationMarkers.visualize() at line 330 where it requires num_markers > 0. Additionally, the PR re-sorts contributor names alphabetically in CONTRIBUTORS.md as required by project guidelines.

Key Changes:

  • Added guard condition in _debug_vis_callback to skip visualization when viz_points.shape[0] == 0
  • Moved 5 contributor names (Anke Zhao, David Leon, Song Yi, Tsz Ki GAO, Weihua Zhang) from the end of the list to their correct alphabetical positions

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The fix is a simple defensive check that prevents errors without changing any existing behavior. The logic is sound - it prevents passing an empty tensor to the visualizer which would fail. The CONTRIBUTORS.md changes are purely administrative and follow project guidelines.
  • No files require special attention

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/sensors/ray_caster/ray_caster.py Added guard to skip visualization when all ray hits are invalid (inf values)
CONTRIBUTORS.md Re-sorted contributor names alphabetically per project guidelines

Sequence Diagram

sequenceDiagram
    participant Event as Simulation Event
    participant RC as RayCaster
    participant Data as ray_hits_w
    participant VM as VisualizationMarkers
    
    Event->>RC: _debug_vis_callback(event)
    RC->>Data: Check if ray_hits_w is None
    alt ray_hits_w is None
        RC->>Event: return early
    else ray_hits_w exists
        RC->>Data: Filter inf values
        RC->>RC: Check if viz_points.shape[0] == 0
        alt No valid points (NEW FIX)
            RC->>Event: return early (prevents error)
        else Has valid points
            RC->>VM: visualize(viz_points)
            VM->>VM: Update point instances
        end
    end
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Fix is correct — the root cause is accurately identified and the guard prevents the ValueError from VisualizationMarkers.visualize() when all rays miss (returning inf). The change is minimal, safe, and non-breaking.

Two observations worth considering:

1. Stale markers persist when all rays miss (minor)

When all hit points are inf and we early-return, the markers from the previous frame remain visible in the viewport. This is cosmetically wrong — the user sees phantom hit points that no longer correspond to reality. Consider calling self.ray_visualizer.set_visibility(False) before returning (and re-enabling at the top of the function), or visualizing an empty set some other way.

This is minor enough that it doesn't block the PR, but it's worth a follow-up.

2. Same bug exists in RayCasterCamera._debug_vis_callback (not in scope, but worth noting)

RayCasterCamera._debug_vis_callback (line 339 on develop) passes self.ray_hits_w.view(-1, 3) directly to visualize() with no inf filtering at all. If all rays miss, it will pass inf values to USD (bad), and if filtering were added it would hit the same zero-count crash. Since the issue reporter specifically mentions MultiMeshRayCaster (which inherits RayCaster and gets this fix), the RayCasterCamera path is a separate but identical bug. Worth a follow-up issue or expanding this PR's scope.

CI: Pre-commit passes. Other checks still pending (just triggered by merge with develop).

Verdict: LGTM with the minor nit about stale markers. The core fix is correct.

# if no points to visualize, skip
if viz_points.shape[0] == 0:
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): When all rays are inf and we return early, the markers from the previous frame remain visible — they're stale. This means the user briefly sees phantom hit points that don't correspond to the current sensor state.

A simple fix would be to hide the visualizer when there's nothing to show:

# if no points to visualize, hide stale markers and skip
if viz_points.shape[0] == 0:
    self.ray_visualizer.set_visibility(False)
    return
else:
    self.ray_visualizer.set_visibility(True)

This is cosmetic and non-blocking — the core fix (preventing the ValueError) is correct. But it would make the debug visualization more accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants