Let weights() store its result as narrower types - #60
Merged
Conversation
A conservative operator is large: a 1000x1000 scene onto a 1000x2000 sensor is 3.93M triples, or 94 MB, and the ESIS Level-4 inversion keeps roughly a terabyte of them. Storing the indices as `int32` and the weights as `float32` halves that, to 47 MB for the same operator. `dtype_indices` and `dtype_values` are separate because their limits are unrelated: the indices are bounded by the number of cells, and `int32` covers about two billion of them, while the weights are bounded by precision. Both default to leaving the result alone. Narrowing is the last step of a build, so the geometry and any merging of repeated pairs are still done in double precision and only the stored result is narrowed. Measured on the operator above, storing the weights as `float32` moves the total weight of a fully covered input cell by 5.2e-8 and a regridded image by 4.7e-8 of its peak, since `regrid_from_weights` accumulates into a double precision array. An index which does not fit raises. Letting it wrap would address the wrong cell and give a plausible answer rather than an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
+ Coverage 99.51% 99.52% +0.01%
==========================================
Files 46 46
Lines 2472 2544 +72
==========================================
+ Hits 2460 2532 +72
Misses 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Checking that every index fits in the requested type cost more than the conversion did: two passes over each array of indices, against one to copy it. On a 3.93M-triple operator that was 16 ms of a 31 ms narrowing. The indices address the flattened grids, so their magnitude cannot exceed the number of cells, which is known without looking at them. When that bound already fits there is nothing to check. The arrays of indices are usually several times larger than the grids they address, so this is most of the cost: narrowing both goes from 31 ms to 24 ms, and narrowing only the indices from 26 ms to 16 ms. The scan still runs, and still reports the true span, when the number of cells does not fit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Adds
dtype_indicesanddtype_valuestoweights(), so a caller can store a large operator asint32/float32instead ofint64/float64.A conservative operator is big. Measured on a 1000x1000 scene resampled onto a 1000x2000 sensor:
int64/float64int32/float32The ESIS Level-4 inversion keeps roughly a terabyte of these, so halving them is worth more there than any speedup — the build is already fast enough after #59, and memory is the binding constraint.
Two keywords, not one
The limits are unrelated, so conflating them into a single "precision" knob would be wrong:
int32covers about 2.1 billion, against 1,978,272 for the sensor above — four orders of magnitude of headroom.Both default to :obj:
None, meaning the result is left alone, so nothing changes for existing callers.Accuracy
Narrowing is the last step of a build. The geometry, and any merging of repeated pairs, are still done in double precision; only the stored result is narrowed.
regrid_from_weightsaccumulates into a double precision array, so the summation stays wide too.Measured on the same operator:
For comparison, accumulating in single precision as well would cost 1.2e-07, so keeping the accumulator wide is worth about a factor of two.
What it costs
Narrowing is plain numpy rather than a compiled kernel, since the work is
astypeand reductions, which are already vectorized. On the operator above, against a 155 ms build withcoalesce=False:That is essentially the cost of the copies, which run near memory bandwidth. It is not free, but it buys half the storage of an operator there may be a terabyte of.
Checking that every index fits used to cost more than the conversion, since it meant two passes over each array of indices against one to copy it. The indices address the flattened grids, so the number of cells bounds them and the scan only runs when that bound does not already fit.
Overflow raises
An index which does not fit in
dtype_indicesraises rather than wrapping around, and the message reports the span and the type's range:A wrapped index addresses the wrong cell and produces a plausible answer instead of an error, which is the failure worth spending a check on. The range is measured from the indices themselves rather than inferred from the grid shape, since the builders can emit negative wraparound indices for descending grids, as
_coalescenotes.Not added to
regrid()regrid()builds its weights, applies them once, and discards them, so narrowing there would cost precision for no saving.Checks
black --check,ruff check, and pyright 1.1.411 all clean🤖 Generated with Claude Code