Skip to content

Follow up fix custom flash block sizes fallback in Ulysses attention#437

Merged
copybara-service[bot] merged 1 commit into
mainfrom
ninatu/fix_heads_per_tile2
Jul 9, 2026
Merged

Follow up fix custom flash block sizes fallback in Ulysses attention#437
copybara-service[bot] merged 1 commit into
mainfrom
ninatu/fix_heads_per_tile2

Conversation

@ninatu

@ninatu ninatu commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

The same fix as in #420 since bug was reintroduced again in #430 during refactoring

@ninatu ninatu requested a review from entrpn as a code owner July 2, 2026 15:03
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Perseus14
Perseus14 previously approved these changes Jul 2, 2026
@ninatu ninatu force-pushed the ninatu/fix_heads_per_tile2 branch 3 times, most recently from da3dbfc to 322fb6b Compare July 8, 2026 09:40
@entrpn

entrpn commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@eltsai can you review these changes?

@entrpn entrpn requested a review from eltsai July 8, 2026 16:08
@ninatu ninatu force-pushed the ninatu/fix_heads_per_tile2 branch 2 times, most recently from 6e416fb to 58b6714 Compare July 9, 2026 07:49
eltsai
eltsai previously approved these changes Jul 9, 2026
# to 1 (the custom-kernel default) to keep the `heads_per_tile > 1` guards safe.
heads_per_tile = getattr(flash_block_sizes, "heads_per_tile", None) or heads_per_tile
vmem_limit_bytes = getattr(flash_block_sizes, "vmem_limit_bytes", None) or vmem_limit_bytes
return bq, bkv, bkv_compute, bkv_compute_in, heads_per_tile, vmem_limit_bytes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we add a if heads_per_tile is None 0 -> heads_per_tile = 1 safety check before return? This PR lgtm otherwise

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I reverted this lines back, now after this block we have

if heads_per_tile is None:
    heads_per_tile = 1

Ensure that block sizes and heads_per_tile fall back to default values
when resolved as None from CustomFlashBlockSizes dataclass. This fixes
a TypeError in ulysses_custom attention when heads_per_tile is
not specified.
@ninatu ninatu force-pushed the ninatu/fix_heads_per_tile2 branch from 58b6714 to b1265c7 Compare July 9, 2026 18:25
@copybara-service copybara-service Bot merged commit 5fe8bcf into main Jul 9, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants