Skip to content

fix(sensors): floor the angles-to-columns map index in integer arithmetic - #174

Merged
janickm merged 1 commit into
NVIDIA:mainfrom
janickm:dev/janickm/angles-to-columns-floor
Sep 2, 2026
Merged

fix(sensors): floor the angles-to-columns map index in integer arithmetic#174
janickm merged 1 commit into
NVIDIA:mainfrom
janickm:dev/janickm/angles-to-columns-floor

Conversation

@janickm

@janickm janickm commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

_init_angles_to_columns_map turns the nearest-neighbour flat element index (column-major, so flat = column * n_rows + row) into a column index by dividing by n_rows and truncating on the cast to the map dtype:

(idxs / self.n_rows).to(self.angles_to_columns_map_dtype)

The division runs in float32, which represents consecutive integers only below 2^24. Above that the quotient rounds up at every flat index just past a multiple of n_rows, moving those cells into the next column:

model elements wrong indices
128 x 3600 (today's Hesai) 460,800 0
512 x 14400 (largest map here) 7,372,800 0
128 x 140000 17,920,000 8,928
64 x 300000 19,200,000 37,856
128 x 200000 25,600,000 68,928

First divergence at flat index 16,777,343: float gives column 131073, exact floor gives 131072.

Not a numpy-version issue

Unlike the sibling fixes in #172 and #173, this is version-independent — the same counts reproduce under torch 2.7.0 and 2.13.0. It is latent headroom, not a live regression.

Today's maps stay well below the limit, so this is behaviour-preserving: the maps for both Hesai models come out bit-identical. It removes the dependency on that headroom and makes the line do what its comment already claimed ("dividing with the total number of rows and flooring").

torch.div's rounding_mode has been available since torch 1.8, well below the 1.12.1 pinned for the python 3.8 toolchain (verified against the actual pinned wheel).

Tests

A model with >2^24 elements would need a >16.7M-point KD-tree, far too expensive for a unit test. So the new test stubs the nearest-neighbour lookup to return flat indices straddling 2^24, which still exercises the real conversion inside _init_angles_to_columns_map. It fails on the unfixed code for all 16 parameterisations.

An earlier version of this test asserted the arithmetic directly rather than going through the model — it passed on the unfixed code, so it would not have caught a revert. The stub version is bound to the implementation.

A second test pins the mapped columns to the model's valid [0, n_columns - 1] range.

Validation

check result
new test vs pre-fix 16 of 16 parameterisations fail
new test with fix all pass
angles_to_columns_map vs pre-fix bit-identical (0/7,372,800 and 0/2,457,600)
//ncore/impl/data:all, //ncore/impl/sensors:all, //tools/... 19 pass, python 3.8 (numpy 1.19.5, torch 1.12.1) and 3.11
numpy 1.26.4 + torch 2.7.0 / numpy 2.3.5 + torch 2.13.0 maps bit-identical, all 614,400 elements pass FOV asserts
bazel run format, ty clean

Downstream

Ran NRE against this branch via --test_env=PYTHONPATH (sentinel-verified the override was live): //libs/vren:lidars_test, //nre/models/gaussians:gsplat_lidar_test, //nre/datasets/samplers:all8 pass.

Rebased onto main after #172 and #173 merged; re-validated there (19 bazel targets, both numpy/torch combinations, maps bit-identical to current main).

@janickm janickm self-assigned this Aug 31, 2026
@janickm
janickm force-pushed the dev/janickm/angles-to-columns-floor branch from ca24a6a to eea8033 Compare August 31, 2026 15:05
…etic

_init_angles_to_columns_map turns the nearest-neighbour flat element index
(column-major, so flat = column * n_rows + row) into a column index by
dividing by n_rows and truncating on the cast to the map dtype. The
division ran in float32, which represents consecutive integers only below
2**24. Above that the quotient rounds up at every flat index just past a
multiple of n_rows, moving those cells into the next column: for a model
with 128 rows and 140000 columns, 8928 of 17920000 indices come out wrong,
and 68928 of 25600000 at 200000 columns.

Today's maps stay well below the limit -- a 128x3600 sensor has 460800
elements, and the largest map here is 7372800 cells -- so this is
behaviour-preserving, and the maps for both Hesai models come out
bit-identical. It removes the dependency on that headroom, and makes the
line do what its comment already claimed.

Unlike the sibling numpy fixes this is not version-dependent: the same
counts reproduce under torch 2.7.0 and 2.13.0. torch.div's rounding_mode
has been available since torch 1.8, well below the 1.12.1 pinned for the
python 3.8 toolchain.

The new test stubs the nearest-neighbour lookup to return flat indices
straddling 2**24, since a model with that many elements would need a
>16.7M-point KD-tree; it exercises the real conversion inside
_init_angles_to_columns_map and fails on the unfixed code for every
parameterisation. A second test pins the mapped columns to the model's
valid range.

Validation: //ncore/impl/data:all, //ncore/impl/sensors:all and
//tools/... pass on both python 3.8 (numpy 1.19.5, torch 1.12.1) and
3.11, and the angles-to-columns maps are unchanged under numpy 1.26.4 +
torch 2.7.0 and numpy 2.3.5 + torch 2.13.0.
@janickm
janickm force-pushed the dev/janickm/angles-to-columns-floor branch from eea8033 to c9d7dcc Compare September 2, 2026 11:55
@janickm
janickm added this pull request to the merge queue Sep 2, 2026
Merged via the queue into NVIDIA:main with commit 59c698d Sep 2, 2026
5 checks passed
@janickm
janickm deleted the dev/janickm/angles-to-columns-floor branch September 2, 2026 12:29
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