[mmporong]

Description

randomize_rigid_body_mass, randomize_rigid_body_inertia, randomize_actuator_gains, randomize_joint_parameters and randomize_fixed_tendon_parameters check their *_distribution_params with _validate_scale_range when operation="scale". That check, added in #3058, treats the parameters as a (low, high) range for every distribution, but with distribution="gaussian" they are (mean, std). The configuration from the issue,

mass_distribution_params=(0.5, 0.1), operation="scale", distribution="gaussian"

fails at term construction with ValueError: mass_distribution_params: upper bound (0.1) must be ≥ lower bound (0.5).

This PR passes the term's distribution (default "uniform", the same default as the term's __call__) to _validate_scale_range at all nine call sites. For gaussian parameters the function now:

• applies the existing lower-bound rules to the mean, so a mean that Adds basic validation tests for scale-based randomization ranges #3058 would reject as a lower bound (for example, a mass mean of 0.0) is still rejected. Gaussian samples are unbounded, so this checks the configuration, not every sampled scale;
• reports a negative standard deviation as standard deviation must be ≥ 0. develop already rejected such a configuration, but as an upper/lower-bound ordering error.

uniform and log_uniform ranges are accepted and rejected as before. The docstrings of the five terms list the new error.

Tested without Isaac Sim, on CPU in an Ubuntu 24.04 Docker container (Python 3.12, torch 2.12.0 CPU wheel, isaaclab, isaaclab_physx and isaaclab_contrib installed in editable mode):

• The new parametrized test_scale_range_validation_follows_distribution in source/isaaclab/test/envs/test_event_requirements.py builds randomize_rigid_body_mass with a SimpleNamespace env. On develop the valid gaussian case fails with the error from the issue, and the negative-std and zero-mean cases fail because develop rejects them with the (low, high) wording; the uniform case passes. With this change all four pass.
• source/isaaclab/test/envs/test_event_requirements.py: 10 passed. source/isaaclab/test/managers/test_event_manager.py: 14 passed.
• The hooks in .pre-commit-config.yaml pass on the changed files (run with pre-commit run --files).

The issue is assigned. If a fix is already in progress there, I am happy to close this one.

This PR was prepared with Claude Code (Claude Opus 5.5): the code change, the test, the verification and this description.

Fixes #3514

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.

Checklist

Docker and GPU tests run on demand. Push the commits you want tested, then
comment run-ci on the pull request.

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[mmporong]

run-ci

[isaaclab-review-bot]

Isaac Lab Review Bot

The change correctly makes scale validation distribution-aware across the five affected event terms, interpreting gaussian parameters as (mean, std) while preserving range validation for uniform and log-uniform distributions. Documentation, changelog, and focused regression coverage accompany the implementation.

• Design and architecture: Validation remains centralized in the existing private _validate_scale_range helper. Each of the nine affected parameter paths forwards the event term’s configured distribution, with the default aligned to the corresponding __call__ default. Gaussian sampling remains unbounded, so validation intentionally constrains the configured mean and standard deviation rather than guaranteeing every sampled multiplier is non-negative.
• API: The private helper gains a keyword-only argument with a default, preserving existing internal call compatibility. Public event-term signatures are unchanged, constructor error documentation is updated, and the package changelog records the corrected gaussian parameter semantics.
• Implementation: The validator preserves existing positivity and zero constraints on the first parameter, retains ordering checks for uniform and log-uniform ranges, and adds the appropriate non-negative standard-deviation check for gaussian parameters. The focused test exercises mass-term construction; the remaining affected paths use the same helper and were traced to ensure they pass their configured distribution consistently.

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.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Fixes parameter validation for randomization functions.

The PR appears safe to merge; broader construction-time coverage would reduce regression risk.

Findings

1.  Other terms lack test coverage ▶

Summary

The PR updates scale-parameter validation to interpret Gaussian parameters as (mean, std) while preserving range checks for uniform distributions, and adds a construction-time regression test.

• Extends the distribution-aware check to five event terms and nine parameter fields.
• Adds documentation and a changelog fragment.

_{Reviews (1) · Last reviewed commit: "Validate gaussian scale randomization pa..."}

[greptile-apps]

Other terms lack test coverage The new cases construct only randomize_rigid_body_mass, but this change updates nine validation calls across five terms. A missed or mismatched call in another term would go unnoticed. Please add construction-time cases for the other terms, including a field that allows zero and a term with multiple distribution parameters.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[mmporong]

run-ci

[StafaH]

This is now resolved by #8127 which does a more broader fix of this issue across the codebase