fix(sensors): floor the angles-to-columns map index in integer arithmetic - #174
Merged
Merged
Conversation
janickm
force-pushed
the
dev/janickm/angles-to-columns-floor
branch
from
August 31, 2026 15:05
ca24a6a to
eea8033
Compare
…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
force-pushed
the
dev/janickm/angles-to-columns-floor
branch
from
September 2, 2026 11:55
eea8033 to
c9d7dcc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_init_angles_to_columns_mapturns the nearest-neighbour flat element index (column-major, soflat = column * n_rows + row) into a column index by dividing byn_rowsand truncating on the cast to the 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: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'srounding_modehas 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.A second test pins the mapped columns to the model's valid
[0, n_columns - 1]range.Validation
angles_to_columns_mapvs pre-fix//ncore/impl/data:all,//ncore/impl/sensors:all,//tools/...bazel run format,tyDownstream
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:all— 8 pass.Rebased onto
mainafter #172 and #173 merged; re-validated there (19 bazel targets, both numpy/torch combinations, maps bit-identical to currentmain).