Fix autoencoder CUDA kernel: initialise example-buffer padding bits#86
Open
drmckay wants to merge 1 commit into
Open
Fix autoencoder CUDA kernel: initialise example-buffer padding bits#86drmckay wants to merge 1 commit into
drmckay wants to merge 1 commit into
Conversation
The kernel computed number_of_literal_chunks but only set the literal bits, leaving the final-chunk padding uninitialised. Zero the whole buffer first, matching the CPU memset.
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.
Fix 2 — CUDA autoencoder kernel leaves padding bits uninitialised
The bug
produce_autoencoder_example(CUDA,cuda/tools.cu) builds the example literal-vectorX, abit-packed array of
number_of_literals = 2*number_of_featuresbits stored innumber_of_literal_chunks = (number_of_literals-1)/32 + 132-bit words. Whennumber_of_literalsis not a multiple of 32, the final word has padding bits
[number_of_literals, chunks*32).The kernel computed
number_of_literal_chunksbut never used it. It initialisedXby clearingthe positive-literal bits one at a time and setting the negated-literal bits — touching only
[0, number_of_literals). The padding bits were never written, so they kept whatever garbagewas in the freshly
mem_alloc-ed device buffer (prepare_X_autoencoderallocatesX_gpubut nevercopies a zeroed host array to it).
The reference: the CPU path already does it right
tmu_produce_autoencoder_example(lib/src/Tools.c):The CUDA kernel was ported from this but dropped the
memset(and replaced it with a positive-bitclear loop). So GPU and CPU diverged, and only the GPU leaves padding uninitialised.
The fix
Match the CPU path: zero all
number_of_literal_chunkswords first, then set the negated-literalbits. Minimal diff in
cuda/tools.cu, and it also silences thenumber_of_literal_chunks declared but never referencednvcc warning.Proof
Correctness is by parity: the CPU reference already zeroes the full buffer; the fix makes the GPU do
the same. Meaningful bits
[0, number_of_literals)are set identically (they were cleared/zeroedeither way); only the padding bits change (garbage → 0).
bench/check_padding.py(branchtest/gpu-ae-padding, RTX 3090): 80 literals, 3 chunks, paddingbits 80..95 →
padding bits set = 0for all 4 targets → PASS — padding deterministically 0.example/toy_autoencoder.py --compare: CPU precision 1.000 / recall 0.358, GPU precision 1.000 /recall 0.398 → no regression (meaningful bits unchanged; only padding went garbage → 0).
number_of_literal_chunks declared but never referenced(now used).