Skip to content

fix: attention mask for MPS#252

Merged
buttercrab merged 1 commit into
nari-labs:mainfrom
pevers:fix-attn-mask-mac
Jun 28, 2025
Merged

fix: attention mask for MPS#252
buttercrab merged 1 commit into
nari-labs:mainfrom
pevers:fix-attn-mask-mac

Conversation

@pevers

@pevers pevers commented Jun 28, 2025

Copy link
Copy Markdown
Contributor

Closes #251

I found the issue. It was because the attention mask changed and now padding tokens could attend to other padding tokens. On CUDA a softmax of -inf will not result in NaN values but on MPS it will be NaN.

I've reverted the mask back to the original mask. Another idea would be to check for MPS and create a different mask. Is there a specific reason to do mask = p_mask_q & p_mask_k instead of filtering out padding masks like before the change?

@buttercrab buttercrab 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.

LGTM

@buttercrab
buttercrab merged commit 998fc25 into nari-labs:main Jun 28, 2025
1 check passed
@pevers
pevers deleted the fix-attn-mask-mac branch June 29, 2025 18:51
longniao pushed a commit to longniao/dia that referenced this pull request Nov 19, 2025
This commit fixes several macOS compatibility issues and updates
documentation to reflect existing MPS (Metal Performance Shaders) support.

Changes:

1. Fix CUDA-specific operations to be properly guarded:
   - dia/model.py: Only call torch.compiler.cudagraph_mark_step_begin() on CUDA devices
   - cli.py: Move cudnn settings inside CUDA availability check
   - app.py: Move cudnn settings inside CUDA availability check

2. Update documentation:
   - README.md: Add comprehensive macOS/Apple Silicon section with usage examples
   - README.md: Update hardware support list to include MPS and CPU
   - README.md: Update TODO to reflect that macOS is already supported
   - README.md: Add performance notes and best practices for macOS users

3. Fix example code:
   - example/simple-mac.py: Use float32 instead of float16 for better MPS stability

Key improvements for macOS users:
- Prevents errors from CUDA-specific API calls on non-CUDA devices
- Documents proper usage: use_torch_compile=False and compute_dtype="float32"
- Clarifies that MPS backend provides GPU acceleration on Apple Silicon

Fixes issues where the code would attempt CUDA operations on macOS, causing
errors or warnings. Documentation now accurately reflects the current state
of macOS support that was added in previous commits (nari-labs#197, nari-labs#252).
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.

Simple demo breaks on Mac with multinomial issue

2 participants