Fix AttnRes silently dropping blocks past 32 - #1344
Open
adityasingh2400 wants to merge 10 commits into
Open
Conversation
_get_max_blocks capped MAX_BLOCKS at 32 for any n_blocks, but MAX_BLOCKS is the static trip count of the tl.static_range loops in both AttnRes kernels. With n_blocks > 32 the forward normalized the depth softmax over only the first 32 blocks and dropped the rest from the weighted sum, and the backward left dV[32:] as the uninitialized torch.empty_like buffer. Round n_blocks up to the next power of two instead. This is byte-identical for every n_blocks <= 32 (next_power_of_2 reproduces the old 4/8/16/32 ladder exactly) and covers all blocks above it. Adds N=40 to the forward and backward correctness parametrizations.
Author
|
Updated the branch onto Worth flagging that no CI has run on this yet. All four workflows have been sitting at
Approving the workflow run is all it needs to produce a signal. |
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.
_get_max_blockscappedMAX_BLOCKSat 32 for anyn_blocks:MAX_BLOCKSis the static trip count of thetl.static_range(0, MAX_BLOCKS)loops in both AttnRes kernels, and the width of the register-heldscoresandalphavectors. So with more than 32 blocks the forward normalized the depth softmax over only the first 32 blocks and dropped the remainder from the weighted sum, and the backward leftdV[32:]as the uninitializedtorch.empty_likebuffer. Both are silent, there is no shape error and no warning.The fix rounds up to the next power of two instead.
MAX_BLOCKSstill has to be a power of two because oftl.arange(0, MAX_BLOCKS), and it still has a floor of 4.This is behavior-preserving for every
n_blocksat or below 32, since rounding to the next power of two reproduces the old ladder exactly: 1, 2, 3 and 4 give 4, then 5 to 8 give 8, 9 to 16 give 16, and 17 to 32 give 32. Only the previously broken range above 32 changes.Added
N=40to the forward and backward correctness parametrizations, which is the smallest case that exceeds the old ceiling.Verification note, stated plainly: these tests need CUDA and I do not have a GPU, so I did not run them. The argument here is by reading, plus the arithmetic above showing the mapping is unchanged at or below 32. Please run the AttnRes suite before merging.