Skip to content

Preserve bounds for built-in observation aggregators - #562

Open
sylvesterkaczmarek wants to merge 1 commit into
google-deepmind:mainfrom
sylvesterkaczmarek:fix/declare-aggregator-bound-preservation
Open

sylvesterkaczmarek wants to merge 1 commit into
google-deepmind:mainfrom
sylvesterkaczmarek:fix/declare-aggregator-bound-preservation

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Fixes #560.

Give built-in observation reducers the preserves_bounds attribute already consumed and documented by Updater.observation_spec(). The misspelled attribute made all built-in reducers appear to have unknown bound behavior, so min, max, mean and median discarded BoundedArray limits.

These four reducers now retain bounds; sum still explicitly discards them. Reduced values, shapes, dtype promotion, unbounded inputs and custom-aggregator handling remain unchanged. No dynamics, rewards, dependencies or workflows change. This is independent of the initial-padding snapshot fix.

Validation

Added 32 cases covering each reducer's declaration, scalar/per-element bounds, integer/float dtype promotion, rejection of values outside retained limits, unbounded sum results, custom true/false/missing declarations, unbounded inputs and unchanged source arrays. An actual native MuJoCo observation sequence exercises mean aggregation through the Updater.

  • New cases against unchanged main: 22 failed, 10 passed.
  • Selected observation, variation, RL and utility suites: 200 passed, 6 camera-related cases deselected, on both Python 3.12.11 and 3.11.16.
  • Both independent observation fixes from this round in a separate worktree: 221 passed, 6 deselected, on both Python versions.
  • Ruff E9/F on both changed files, Pyink on the added tests and changed source line, compilation and git diff --check: passed.

Based on main at a04e3e4cf56c12117d2294bb090f9acec21e5c67. Tested on macOS with MuJoCo 3.14.0, NumPy 2.3.2 and SciPy 1.16.1; bindings were generated using build_mjbindings. Rendering was disabled. A broader run failed the same three camera-rendering tests on unchanged main and both branches; the CPU selection above excludes camera-related cases only, without modifying repository test settings. The existing axis-angle warning is unchanged. The complete repository suite, rendering, training runs, package builds and other operating systems were not tested. This corrects metadata for the existing reducers rather than changing reduction algorithms or validating arbitrary user-defined reducers.

Upstream checks

At submission the CLA passed, the GitHub Actions Scan was queued, and the organization security workflow reported action_required: https://github.com/google-deepmind/dm_control/actions/runs/36484669610. The test counts above are local results, not completed upstream CI.

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

@saran-t This observation-layer PR is current with main, has no failing or pending public checks, and has not yet received human review. It preserves BoundedArray bounds through the built-in observation aggregators and includes focused bounds regressions. Could you review the current head when convenient?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Built-in observation aggregators drop bounds-preserving specifications

1 participant