Repository navigation
Transpose weights conservatively on a CUDA device - #69
Merged
Merged
Conversation
… there. `transpose_weights_conservative()` brought device weights back to the host and returned host values beside device indices, which neither path of `regrid_from_weights()` could apply. Weights built with `device="cuda"` are now normalized by a kernel, and the index arrays are swapped without being copied, as on the host. Empty slots keep their index of -1, now on the output side, where the device scatter already skips it. The cell volumes and `weights_input` are still computed on the host from the grids, and are now broadcast along the orthogonal axes rather than copied out, so that each distinct row is sent to the device only once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
The sweep which computes them is a `prange` loop, but it was inlined into a function compiled without `parallel=True`, so it ran on one thread. That made it two thirds of the time of a transpose on the device: for 35 million weights, 0.56 s of 0.70 s. In parallel the areas take a quarter of the time, and the transpose takes 0.23 s on the device and 0.51 s on the host, from 0.70 s and 0.90 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
==========================================
+ Coverage 99.68% 99.71% +0.03%
==========================================
Files 62 65 +3
Lines 3835 4251 +416
==========================================
+ Hits 3823 4239 +416
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:
|
Correctness: - A dimensionless `weights_input` with a scale, such as a percentage, is reduced to the number it stands for on both sides of the transpose, as `weights()` reduces it on a device. Before, the device was wrong by the scale squared and the host by the scale. - A weight which addresses a cell outside the grids raises ValueError on both paths, rather than an IndexError on the host and a read past the end of the volumes on the device. - `weights_input` is converted to double precision before it is squared, so a single-precision one gives the same result on the host and device. Performance: - The volumes of each grid are computed only along the orthogonal axes it varies along, so a grid shared by every element of the other is computed and sent once, rather than once for each element. - The volume of each input cell is divided by the square of its weight once, on the host, rather than for every weight which touches it, so the kernel reads one factor instead of two and needs no placeholder for a missing `weights_input`. - The cell areas are computed in parallel across the grids from eight grids on, and within each grid below that. Within each grid alone, 5000 small grids took 43 times as long as computing them serially. - `to_device_cached()` in `regridding._cuda` replaces the copies of the same upload cache in this and in `convolve_weights()`. The device tests use their own grids, whose input cells differ in area from one element to the next, and a `weights_input` which varies along the orthogonal axis as well, so that a row sent for the wrong element fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
- The grids are checked against the shape the weights were built for, once, before anything is read. Before, only the largest index was compared with the number of cells, so grids with as many cells or more, such as ones with their axes swapped, were read at the wrong cells without failing, and every element's indices were scanned twice on the host to do it. - Axes along which the grids or `weights_input` have been broadcast are found from their strides, so grids which the caller broadcast first, as named-arrays always does, have their volumes computed once. Through ctis, the sensor grid's areas were computed for each of the 80 channels and wavelengths, and are now computed once. - The device is sent the volumes once, as they are before being broadcast, and each element reads its own row of them there, instead of a cache of rows keyed on host memory. `convolve_weights()` sends its kernels the same way, and `to_device_cached()` is gone. - The cell areas of every grid are one parallel loop over every line of edges of every grid, rather than one of two strategies chosen by a threshold measured on one machine. They are the same areas, to the bit, as `grid_volume()` computes for each grid alone, and as fast as the better of the two strategies or faster. - `_util._dimensionless()` reduces a dimensionless quantity to the number it stands for, in double precision before it is scaled, for `weights()`, `convolve_weights()` and the transpose alike. A single-precision percentage is now the same on the host and the device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
- `_cuda.rows()` reduces every orthogonal axis with a stride of zero to a length of one before sending an array, and returns which row each element reads as an array numpy broadcast, in place of `row()`. A kernel which the caller broadcast along an orthogonal axis is sent to the device once by `convolve_weights()` again, as main sent it, rather than once for each element. - The transpose finds the axes `weights_input` was broadcast along before it converts it to double precision or from a unit, which copied it out to its full shape, so it is sent once rather than once for each element. - `convolve_weights()` reports a kernel which is not numbers as numpy does, rather than as one with the wrong unit, and both unit errors keep their cause. - The grids are checked before their orthogonal axes are broadcast against the weights', which now raises its own error, and the message for grids of the wrong size says that the weights should be conservative ones, as weights built by the multilinear method are refused by the same check. - An orthogonal axis of length zero transposes into no weights again, rather than failing to reshape. - `convolve_weights()` only broadcasts the kernel along the orthogonal axes on the host, where it is read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
- A `weights_input` of zero in a cell which no weight touches no longer warns. Folding it into the volume of every cell divided by it everywhere, where main divided only at the cells the weights touch; a cell which one does still warns, when its weights are scaled. - `_util._dimensionless()` converts an array which has been broadcast as it was before, and broadcasts it again, and skips a scale of one, so no caller copies a broadcast array out to its full shape. A kernel in single precision or with a unit which the caller broadcast along an orthogonal axis is sent to the device once by `convolve_weights()`. It takes the name to give in its error, which `convolve_weights()` used to wrap it for. - `_util._unbroadcast()` is the one place which finds the axes along which arrays have been broadcast, for the transpose, `_dimensionless()` and `_cuda.rows()`. - Grids with more resampled axes than the weights have axes raise ValueError rather than IndexError, and the hint in the grid check names every likely cause: the grids, the axes, weights which are not conservative, and weights transposed since. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU
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.
Lets
transpose_weights_conservative()take weights built withdevice="cuda"and leave its result on the device, so a forward model and its transpose can both stay there.Before
Device weights were brought back to the host. The function then returned host values beside device indices, which neither path of
regrid_from_weights()can apply:Device path
regridding/_weights/_weights_transposed/_cuda.py:float64, as on the host.-1, now on the output side. The device scatter already skips that side (since Addconvolve_weights()to fold a point-spread function into a set of weights #67). Their values are set to zero rather than read._cuda.rows()also reduces any axis the caller already broadcast (a stride of zero) before sending.convolve_weights()now sends its kernels the same way, so a kernel the caller broadcast is still sent once, as on main.ValueError.transpose_weights()already worked on device weights. Its docstring now says so, and a test covers it.Shared by host and device
ValueErrorinstead of being read at the wrong cells. The message names the likely causes: the wrong grids or axes, non-conservative weights such as multilinear ones (which address vertices), or weights transposed since without swapping the grids. Grids with more resampled axes than the weights have axes raise the same way. Grids whose orthogonal axes cannot broadcast against the weights' raise their ownValueError.weights_inputsquared once, on the host, rather than for every weight that touches it. With aweights_input, host results therefore differ from main by up to about 4e-16 relative, from the order of operations; without one they are identical. A zeroweights_inputin a cell no weight touches doesn't warn, as on main._util._dimensionless()reduces a unitless quantity with a scale, such as50 %, to the number it stands for, converting tofloat64before scaling. An array the caller broadcast is converted at its own size and broadcast again, so no caller copies it out. It servesweights(),convolve_weights()and the transpose._util._unbroadcast()is the one place that finds broadcast axes, for the transpose,_dimensionless()and_cuda.rows(). Before, the host transpose was wrong by the scale (100 for percent), and the device would have been wrong by its square. A unit with dimensions is dropped by the transpose, as before.weights_inputis converted tofloat64before it is squared, so afloat32one gives the same result on both paths. Its broadcast axes are found before that conversion, which would otherwise copy it out to its full shape.Parallel cell areas
The sweep that computes the cell areas is a
prangeloop inlined into_cell_volume_2d, which was compiled withoutparallel=True, so it ran on one thread. Now each sweep is one parallel loop over every line of edges of every grid. Its areas are bit for bit those ofgrid_volume()on each grid alone. Many small grids cost no more to start than one large one, and a few large grids still use every thread._sweep.weights_conservative_2dcalls the same inlined sweep withoutparallel=True, so the forward host build still computes its areas on one thread. Fixing that is left for a separate PR.Timings
Warm, on an RTX 4090 and 48 host threads.
MART end to end: ctis's
MartInverteron the host, against the same algorithm on the device, using device weights and torch for the rest. The device result matches ctis to 1e-14, with the same iteration count and χ².The device's time per iteration barely changes with size, because launching one kernel per orthogonal element dominates it. One launch for all elements would be the next lever.
Tests
_weights_transposed/_cuda_test.py(markedcuda). Its input grids differ in cell area from one element to the next, so a row read for the wrong element fails.weights_input, one shared by every element, one that varies per element, and afloat32one.int32indices are kept and values becomefloat64.weights_inputgives the same result as its number; mismatched grids raise before anything is sent; corrupted weights raise.float32or percentweights_input. Convolved weights with more orthogonal elements than their grids match the host. In_weights_convolved/_cuda_test.py, a kernel the caller broadcast is sent once, in double or single precision or as a percentage.transpose_weights()works on device weights.In
_weights_transposed_test.py:grid_volume()bit for bit for 1, 2 and 9 grids, and a stacked transpose matches each grid transposed alone.weights_inputdoesn't warn. In_weights_convolved_test.py, a kernel that isn't numbers raises numpy's own error.Local runs:
regridding/_tests/test_util.pycovers_unbroadcast()and_dimensionless().NUMBA_DISABLE_JIT=1.🤖 Generated with Claude Code
https://claude.ai/code/session_01GqceVSp8s2msxj1Dr9MrGU