Skip to content

Keep normalization results floating point for integer inputs - #758

Open
catplotlib wants to merge 1 commit into
NVIDIA:mainfrom
catplotlib:normalization-integer-dtype
Open

catplotlib wants to merge 1 commit into
NVIDIA:mainfrom
catplotlib:normalization-integer-dtype

Conversation

@catplotlib

Copy link
Copy Markdown

Changes proposed in this pull request:

  • Give the normalization primitives in gr00t/data/utils.py a floating-point result dtype, so integer inputs are no longer silently truncated.
  • Add tests/gr00t/data/test_normalization_values.py, the first direct coverage for these four functions.

The bug

normalize_values_minmax, normalize_values_meanstd and unnormalize_values_meanstd size their output with np.zeros_like(values), which inherits an integer input dtype. The division is then truncated on assignment, so an integer state or action array is silently corrupted rather than normalized:

import numpy as np
from gr00t.data.utils import normalize_values_minmax, normalize_values_meanstd

values = np.array([[5, 7]], dtype=np.int64)

print(normalize_values_minmax(values, {"min": np.array([0.0, 0.0]), "max": np.array([10.0, 10.0])}))
# [[-1 -1]]          expected [[0.0, 0.4]]

print(normalize_values_meanstd(values, {"mean": np.array([4.0, 4.0]), "std": np.array([3.0, 3.0])}))
# [[0 1]]            expected [[0.333, 1.0]]

For min/max every value collapses to -1, because the truncated 0 then goes through 2 * 0 - 1. There is no warning; training or inference would just consume wrong numbers.

Passing the same values as float64 gives the correct results, so the behaviour depends entirely on the caller's dtype. StateActionProcessor forwards state[joint_group] and action[joint_group] straight into these functions without casting, and only one dataset path (sharded_single_step_dataset.py) casts to float32 on the way in.

To be clear about severity: every dataset shipped in demo_data/ declares dtype=float32, so I do not believe this is reachable through the datasets in this repo today. It is a latent trap for anyone feeding integer-valued state or action features — discrete grippers, mode flags, indices.

The fix uses np.result_type(values, np.float32), which keeps float32 inputs as float32 (no upcast on the common path) and promotes integers to float64.

Tests

normalize_values_* had no direct tests, so the new file covers the primitives rather than only the regression:

  • min/max and mean/std normalize → unnormalize roundtrips
  • the documented degenerate cases (min == max → 0, std == 0 → pass through)
  • per-step 2D bounds and batched (B, T, D) inputs
  • result dtype: integer inputs not truncated, float32/float64 preserved
pytest tests/gr00t/data/test_normalization_values.py -q

14 passed with this change. With the utils.py change reverted, the 3 dtype tests fail and the other 11 still pass, so the new coverage documents existing behaviour and only the regressions are new.

Wider suite on this branch: pytest tests/gr00t/data/ -m "not gpu" gives 213 passed. The 3 failures in test_dataset_factory.py and the collection error in test_embodiment_tags.py are pre-existing — they reproduce identically with this change stashed, and both come from an unrelated non-default argument 'diffusion_model_cfg' follows default argument error in my environment.

No new dependencies: the tests use seeded numpy rather than adding a property-testing library.

Before submitting

  • I've read and followed all steps in the Making a pull request section of the CONTRIBUTING docs.
  • I've updated or added any relevant docstrings.
  • If this PR fixes a bug, I've added a test that will fail without my fix.
  • If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

normalize_values_minmax, normalize_values_meanstd and
unnormalize_values_meanstd size their output with np.zeros_like(values),
which inherits an integer input dtype. The subsequent division is then
truncated on assignment, so an integer state or action array is silently
corrupted rather than normalized:

    values [[5, 7]] in range [0, 10]
      minmax  -> [[-1, -1]]   expected [[0.0, 0.4]]
      meanstd -> [[ 0,  1]]   expected [[0.333, 1.0]]

Every value collapses to the truncated result, with no warning. Pick a
float result type instead, preserving float32 inputs as float32 so the
common path keeps its dtype.

Add direct tests for the four normalization primitives, which had no
coverage: roundtrips for both schemes, the documented degenerate cases
(min == max, std == 0), per-step 2D bounds, batched inputs, and the
integer-dtype regressions above.
@catplotlib
catplotlib force-pushed the normalization-integer-dtype branch from 4affbb9 to a6287cd Compare August 24, 2026 01:35
@catplotlib

Copy link
Copy Markdown
Author

Bumping this one — CI is green. Anything I can do to help move it along, or anyone I should tag?

@catplotlib

Copy link
Copy Markdown
Author

Following up once more on this one. CI is green and the change is contained to the normalization helper, so it should be cheap to review.

If it isn't a priority or the approach isn't what you want, that's completely fine - just say and I'll close it.

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.

1 participant