Skip to content

[fix] avoid too small block m/n for flex attention#1572

Merged
AlpinDale merged 1 commit into
mainfrom
flex-pool
Nov 4, 2025
Merged

[fix] avoid too small block m/n for flex attention#1572
AlpinDale merged 1 commit into
mainfrom
flex-pool

Conversation

@AlpinDale

Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: AlpinDale <alpindale@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a fix to prevent FlexAttention block sizes (block_m and block_n) from becoming too small by enforcing a lower bound of 16. This is a good fix to improve the stability of the attention kernel. My review focuses on improving the code's maintainability by replacing the magic number 16 with a named constant, which makes the code clearer and easier to manage in the future.

return kernel_options
else:
preferred_block = 32 if query.dtype == torch.float32 else 64
block_lower_bound = 16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The value 16 is a magic number. To improve readability and maintainability, it should be defined as a constant with a descriptive name in UPPER_CASE_WITH_UNDERSCORES as per PEP 8. This makes the code's intent clearer and future updates easier. The suggestion below introduces a constant and assigns it to block_lower_bound for compatibility with the rest of the code in this PR. Ideally, block_lower_bound should be replaced with the constant MIN_KERNEL_BLOCK_SIZE directly on lines 850-851.

Suggested change
block_lower_bound = 16
MIN_KERNEL_BLOCK_SIZE = 16
block_lower_bound = MIN_KERNEL_BLOCK_SIZE

@AlpinDale AlpinDale merged commit 4cd7937 into main Nov 4, 2025
1 check passed
@AlpinDale AlpinDale deleted the flex-pool branch November 4, 2025 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant