Skip to content

Conversation

@LoserCheems
Copy link
Collaborator

Replaces hardcoded True value with proper mask evaluation logic to correctly determine if any elements in the mask are active, ensuring accurate conditional processing in the attention mechanism.

Replaces hardcoded True value with proper mask evaluation logic to correctly determine if any elements in the mask are active, ensuring accurate conditional processing in the attention mechanism.
Copilot AI review requested due to automatic review settings July 3, 2025 14:01
@LoserCheems LoserCheems merged commit c99e567 into main Jul 3, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR corrects the mask validation in the forward triton kernel by replacing a hardcoded True with a proper check that determines if any mask elements are active.

  • Restores the original tl.sum(mask > 0) > 0 logic for any_active.
  • Removes the hardcoded True assignment.
  • Ensures conditional processing in the attention mechanism is accurate.
Comments suppressed due to low confidence (1)

flash_dmattn/flash_dmattn_triton.py:135

  • Add a unit test for the scenario where the mask has no active elements to verify that any_active correctly evaluates to False and that the attention mechanism handles this case as expected.
        any_active = tl.sum(mask > 0) > 0

# Check if any element in mask is non-zero
# any_active = tl.sum(mask > 0) > 0
any_active = True
any_active = tl.sum(mask > 0) > 0
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the commented-out legacy code now that the proper mask check is active to reduce clutter.

Copilot uses AI. Check for mistakes.
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.

2 participants