Support root_height_below_minimum for rough terrain

[hany606]

Description

Similar to rewards.base_height_l2

IsaacLab/source/isaaclab/isaaclab/envs/mdp/rewards.py

Lines 100 to 122 in 7e8ebe6

	def base_height_l2(
	env: ManagerBasedRLEnv,
	target_height: float,
	asset_cfg: SceneEntityCfg = SceneEntityCfg("robot"),
	sensor_cfg: SceneEntityCfg | None = None,
	) -> torch.Tensor:
	"""Penalize asset height from its target using L2 squared kernel.
	Note:
	For flat terrain, target height is in the world frame. For rough terrain,
	sensor readings can adjust the target height to account for the terrain.
	"""
	# extract the used quantities (to enable type-hinting)
	asset: RigidObject = env.scene[asset_cfg.name]
	if sensor_cfg is not None:
	sensor: RayCaster = env.scene[sensor_cfg.name]
	# Adjust the target height using the sensor data
	adjusted_target_height = target_height + torch.mean(sensor.data.ray_hits_w[..., 2], dim=1)
	else:
	# Use the provided target height directly for flat terrain
	adjusted_target_height = target_height
	# Compute the L2 squared penalty
	return torch.square(asset.data.root_pos_w[:, 2] - adjusted_target_height)

I have added the support of rough terrains to terminations.root_height_below_minimum

Fixes #3986

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

[greptile-apps]

Greptile Overview

Greptile Summary

This PR attempts to enhance the root_height_below_minimum termination function to support rough terrain, similar to the existing rewards.base_height_l2 function. The change aims to address issue #3986 where the termination function only worked for flat terrains. The implementation adds RayCaster sensor support to calculate asset height relative to the terrain surface rather than absolute world height. However, the current implementation has a critical syntax error where sensor_cfg is referenced in the function body but is not declared as a parameter in the function signature.

Important Files Changed

Filename	Score	Overview
source/isaaclab/isaaclab/envs/mdp/terminations.py	1/5	Added rough terrain support to root_height_below_minimum function but contains critical syntax error with undeclared sensor_cfg parameter

Confidence score: 0/5

• This PR is not safe to merge and will immediately cause syntax errors and runtime failures
• Score reflects a critical implementation flaw where sensor_cfg parameter is used but not declared in the function signature, making the code syntactically invalid and breaking all calls to this function
• Pay close attention to the terminations.py file which requires immediate fixing of the missing parameter declaration before this can be merged

Sequence Diagram

sequenceDiagram
    participant User
    participant Environment as "ManagerBasedRLEnv"
    participant Asset as "RigidObject"
    participant Sensor as "RayCaster"
    participant Termination as "root_height_below_minimum"
    
    User->>Environment: "Step environment"
    Environment->>Termination: "Check termination conditions"
    Termination->>Asset: "Get root position (root_pos_w)"
    Asset-->>Termination: "Return world position [x, y, z]"
    Termination->>Termination: "Extract height (z-coordinate)"
    
    alt Rough terrain with sensor
        Termination->>Environment: "Get sensor from scene"
        Environment-->>Termination: "Return RayCaster sensor"
        Termination->>Sensor: "Get ray hits data (ray_hits_w)"
        Sensor-->>Termination: "Return ray hit positions"
        Termination->>Termination: "Calculate mean terrain height"
        Termination->>Termination: "Adjust asset height by subtracting terrain height"
    end
    
    Termination->>Termination: "Compare adjusted height < minimum_height"
    Termination-->>Environment: "Return termination tensor (boolean)"
    Environment-->>User: "Episode terminated if below minimum"

Loading

Additional Comments (1)

1. source/isaaclab/isaaclab/envs/mdp/terminations.py, line 62-64 (link)

   syntax: Missing sensor_cfg parameter in function signature. The parameter is used on line 73 but not declared.

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR enhances the root_height_below_minimum termination function to support rough terrain environments by adding an optional sensor_cfg parameter. When provided, the function uses RayCaster sensor data to measure local terrain height and adjusts the asset's height calculation relative to the terrain surface rather than using absolute world coordinates. This change mirrors the pattern established in the rewards module's base_height_l2 function and fixes issue #3986 where assets in complex terrains like Inverted Pyramid Stairs were incorrectly terminated due to their starting positions being below the flat terrain reference level. The modification enables proper termination behavior in environments with varying terrain heights by making height measurements terrain-relative.

Important Files Changed

Filename	Score	Overview
source/isaaclab/isaaclab/envs/mdp/terminations.py	3/5	Added optional sensor_cfg parameter to support rough terrain height calculations using RayCaster sensor data

Confidence score: 3/5

• This PR requires careful review due to potential robustness issues in the sensor data handling implementation
• Score reflects missing validation checks for sensor data availability, use of potentially outlier-sensitive torch.mean, and lack of error handling for edge cases
• Pay close attention to the terminations.py file, particularly the sensor data processing logic and consider adding validation for sensor availability

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds support for rough terrains to the root_height_below_minimum termination function, which was previously limited to flat terrains only. The change enables the termination function to properly handle scenarios where the robot operates on uneven terrain by incorporating terrain height data into the height comparison logic. This enhancement brings consistency with similar functionality already implemented in the reward system (rewards.base_height_l2). The contributor has also been added to the CONTRIBUTORS.md file following standard open source practices.

Important Files Changed

Filename	Score	Overview
CONTRIBUTORS.md	5/5	Added "Hany Hamed" to the alphabetically sorted contributor list

Confidence score: 5/5

• This PR is safe to merge with minimal risk as it addresses a specific limitation with clear intent
• Score reflects a straightforward administrative change with proper contributor acknowledgment
• No files require special attention as the main functional changes appear to be missing from the provided context

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR adds support for rough terrain to the root_height_below_minimum termination function in IsaacLab's MDP module. The change introduces an optional sensor_cfg parameter that allows the function to use RayCaster sensor data to calculate asset height relative to the terrain surface, rather than using absolute world coordinates. This follows the same pattern as the existing rewards.base_height_l2 function, providing consistent terrain-aware height calculations across the codebase.

The modification addresses a limitation where the termination function only worked on flat terrains. In rough terrain scenarios like Inverted Pyramid Stairs, assets start from bases placed below the flat terrain level, making absolute world height comparisons ineffective. With this change, when a sensor configuration is provided, the function adjusts the asset height by subtracting the mean height of the sensor's ray hits from the asset's world position, enabling proper height-based terminations on uneven surfaces.

Important Files Changed

Filename	Score	Overview
CONTRIBUTORS.md	5/5	Added "Hany Hamed" to the contributors list in alphabetical order
source/isaaclab/isaaclab/envs/mdp/terminations.py	4/5	Added RayCaster support and sensor_cfg parameter to root_height_below_minimum function for rough terrain compatibility

Confidence score: 4/5

• This PR makes a focused change that addresses a specific terrain-handling limitation with minimal risk
• Score reflects well-structured implementation following existing patterns, though missing comprehensive tests for the new functionality
• Pay close attention to the terminations.py file to ensure the sensor data handling logic is correct and robust

Sequence Diagram

sequenceDiagram
    participant User
    participant "ManagerBasedRLEnv" as Env
    participant "RigidObject" as Asset
    participant "RayCaster" as Sensor
    participant "torch" as Torch

    User->>Env: "Call root_height_below_minimum(env, minimum_height, asset_cfg, sensor_cfg)"
    Env->>Asset: "Get asset from scene[asset_cfg.name]"
    Asset->>Env: "Return asset object"
    Env->>Asset: "Get asset.data.root_pos_w[:, 2]"
    Asset->>Env: "Return asset height (z-coordinate)"
    
    alt sensor_cfg is not None
        Env->>Sensor: "Get sensor from scene[sensor_cfg.name]"
        Sensor->>Env: "Return RayCaster sensor object"
        Env->>Sensor: "Get sensor.data.ray_hits_w[..., 2]"
        Sensor->>Env: "Return ray hit z-coordinates"
        Env->>Torch: "Calculate mean(ray_hits_w[..., 2], dim=1)"
        Torch->>Env: "Return mean sensor readings"
        Env->>Env: "Adjust asset_height = asset_height - mean_sensor_readings"
    end
    
    Env->>Torch: "Compare asset_height < minimum_height"
    Torch->>Env: "Return boolean tensor"
    Env->>User: "Return termination condition tensor"

Loading

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}