Fix missed rays in base height reward

[AntoineRichard]

Description

RayCaster uses non-finite hit positions when rays miss the configured terrain mesh. The base height reward previously averaged those values directly, causing a single missed ray to produce an infinite penalty and destabilize training.

This change ignores non-finite ray hits when estimating terrain height and returns a neutral penalty when no valid terrain height is available. The implementation does not mutate the shared sensor buffer and preserves the existing behavior for finite scans and flat terrain.

Fixes #1928

Type of change

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

Release backport

• Backport this pull request to the active release branch after it merges into develop

Screenshots

Not applicable.

Testing

• Focused MDP reward and environment unit tests: 5 passed
• Full pre-commit suite: passed
• Verified both new regression tests fail against the previous implementation
• Benchmarked equivalent GPU reductions and selected the fastest measured form

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
• I have added a changelog fragment for every touched package
• My name already exists in CONTRIBUTORS.md

[greptile-apps]

Greptile Summary

This PR makes the sensor-aware base-height reward ignore non-finite ray hits and return a neutral penalty when no terrain height can be estimated.

• Replaces the direct ray-height mean with a non-finite-safe reduction.
• Preserves the shared RayCaster buffer while producing one penalty per environment.
• Adds regression coverage for mixed invalid hits, all-invalid scans, and buffer immutability.
• Adds a changelog entry describing the corrected reward behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed reduction matching its documented behavior and regression coverage.

The implementation filters both infinite and NaN ray heights without mutating shared sensor data, returns the required batched reward shape, and preserves finite-scan and sensor-free behavior.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/envs/mdp/rewards.py	Safely excludes non-finite ray heights, neutralizes all-invalid environments, and preserves the existing flat-terrain branch.
source/isaaclab/test/envs/test_mdp_rewards.py	Adds focused tests covering mixed finite and non-finite hits, all-invalid scans, and non-mutation of sensor data.
source/isaaclab/changelog.d/antoiner-fix-base-height-ray-misses.rst	Accurately documents the corrected handling of missed terrain rays.

_{Reviews (1): Last reviewed commit: "Handle missed rays in base height reward" | Re-trigger Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

Reviewed the base_height_l2 handling of non-finite ray-caster hits, including finite-scan compatibility, all-invalid scans, and sensor-buffer ownership. The change is contained and supported by focused regression tests and a package changelog fragment.

• Design and architecture: Filtering invalid hits within the reward avoids changing RayCaster semantics or mutating its shared buffer. The documented tradeoff is that an entirely invalid scan produces a neutral zero penalty rather than estimating terrain height through a fallback.
• API: The function signature and sensor-free behavior remain unchanged. The changed semantics—ignoring non-finite hits and returning zero when no terrain height is available—are explicitly documented without removing or renaming public API.
• Implementation: The out-of-place mask preserves the sensor data, nanmean retains valid rays while excluding infinities and existing NaNs, and all-invalid rows are neutralized after reduction. Tests cover mixed invalid hits, fully invalid scans, and non-mutation of the shared ray-hit tensor.

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.

[kellyguo11]

@StafaH @ooctipus would be good to get your review on this

[ooctipus]

instead of doing the magic in the code, why not expose a nan handling argument in the input arguments, maybe like convert_inf: float = 0.0

[ooctipus]

do we really need convert inf to nan, the nan to 0.0... also 0.0 for missing terrain might not be idea, I'd imagine you want to bound inf to something like 10m or -inf to -10m instead of 0

[AntoineRichard]

Taking a bit more time to read through all this, I agree with you @ooctipus. I think the issue, and this PR are taking the wrong route. Having -inf is expected if the object is not ontop of the terrain. The main question is should it be -inf, or something very large so that the network do not get NaNs. But maybe the user should use a different MDP in that case. I'm closing this PR. and I'll reply accordingly and close the attached issue.