[Sonica-B]

Description

RewardClipping defaults clamp_min and clamp_max to None, RewardClippingConfig exposes
both as optional, and _apply_transform already has a branch for each one-sided case. The
constructor, however, calls torch.as_tensor on both values, so RewardClipping(clamp_max=1.0)
and RewardClipping(clamp_min=0.0) raise RuntimeError: Could not infer dtype of NoneType.

This PR:

• keeps a missing bound as None (registered as a None buffer);
• bounds the open side of the transformed reward spec by the extreme value of the reward dtype,
  as ClipTransform does, so check_env_specs passes;
• prints None for a missing bound in __repr__ instead of calling float(None), and prints
  the bounds to 4 significant figures (:.4g);
• raises ValueError when neither bound is given (same message style as ClipTransform;
  RewardClipping() raised RuntimeError before);
• fixes the docstring, which documented clip_min / clip_max instead of the real argument
  names, documents in_keys / out_keys, and adds an example.

Two-sided clipping is unchanged.

flowchart LR
    subgraph Before
        A1["RewardClipping(clamp_max=1.0)"] --> B1["torch.as_tensor(None)"] --> C1["RuntimeError"]
    end
    subgraph After
        A2["RewardClipping(clamp_max=1.0)"] --> B2["clamp_min buffer = None"]
        B2 --> C2["_apply_transform: reward.clamp_max(1.0)"]
        B2 --> D2["reward spec: Bounded(finfo.min, 1.0)"]
    end

Loading

Output of the same script on main and on this branch (CPU, Windows):

== main dab6b8c
{'clamp_max': 1.0} -> RuntimeError: Could not infer dtype of NoneType
{'clamp_min': 0.0} -> RuntimeError: Could not infer dtype of NoneType
== branch a17935e
{'clamp_max': 1.0} -> [-3.0, 0.5, 1.0] | spec low/high: -3.4028234663852886e+38 1.0
{'clamp_min': 0.0} -> [0.0, 0.5, 2.0] | spec low/high: 0.0 3.4028234663852886e+38

Motivation and Context

One-sided reward clipping (for example, flooring large negative penalties) is advertised by the
signature and by RewardClippingConfig, but has never worked. The linked issue has the minimal reproduction.

• I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Documentation (update in the documentation)

Checklist

• I have read the CONTRIBUTION guide (required)
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.

Testing

• TestRewardClipping.test_one_sided_bound (lower-only and upper-only): clips four hand-picked
  rewards on one side only, then checks the reward spec bounds of a TransformedEnv and runs
  check_env_specs. test_no_bound_raises checks the ValueError. All three cases fail on main
  with RuntimeError: Could not infer dtype of NoneType and pass on this branch. test_repr
  checks the printed bounds (4 significant figures, None for a missing bound).
• pytest test/transforms/test_reward_transforms.py -k TestRewardClipping: 16 passed, 1 skipped
  (the existing "no inverse" skip), on Windows 11, Python 3.11, torch 2.14.1+cpu,
  tensordict 0.15.3. The whole file: 285 passed, 7 skipped.
• pre-commit run --files on both files: all hooks pass (autoflake and
  check-docstring-args run by hand, since their entries expect Linux executable names).
• Not run here: the Linux CI matrix and GPU jobs.

Fixes #4621

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/4622

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 16 Awaiting Approval

As of commit 31b4786 with merge base dab6b8c ():

AWAITING APPROVAL - The following workflows need approval before CI can run:

• Build Aarch64 Linux Wheels (gh)
• Build Linux Wheels (gh)
• Build M1 Wheels (gh)
• Build Windows Wheels (gh)
• Continuous Benchmark (PR) (gh)
• Examples Tests on Linux (gh)
• Generate documentation (gh)
• Habitat Tests on Linux (gh)
• Libs Tests on Linux (gh)
• Lint (gh)
• LLM Tests on Linux (gh)
• Push Binary Nightly (gh)
• SOTA Tests on Linux (gh)
• Tutorials Tests on Linux (gh)
• Unit-tests on Linux (gh)
• Validate Test Partitioning (gh)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[coder-jayp]

LGTM, thank you for the contribution!

The fix cleanly addresses single-sided clipping across the constructor, spec transformation, and string representation, while adding proper validation and docstring updates. The regression tests in test_reward_transforms.py cover both single-bound configurations and spec bounds thoroughly.

[theap06]

I think it would be better to have this as a ValueError instead.

[Sonica-B]

Done in 31b4786: RewardClipping() with no bound now raises ValueError, and test_no_bound_raises checks for it.

[theap06]

I'd prefer to stick with significant figures so can we use :.4g. cc @bsprenger what do you think?

[Sonica-B]

Done in 31b4786: __repr__ now formats both bounds with :.4g (so clamp_max=1e-5 prints as 1e-05 instead of 0.0000), and a new test_repr covers it.

[theap06]

Thanks for contributing! Few nitpicks on my end but overall looks great!