-
Notifications
You must be signed in to change notification settings - Fork 820
[All] Guard THD learnable dSink on older cuDNN #3470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| from ..attention import is_fused_attn_kernel_available, make_swa_mask, canonicalize_attn_mask_type | ||
| from ..attention import fused_attn | ||
| from ..attention import CPStrategy | ||
| from ..cpp_extensions.misc import get_cudnn_version | ||
| from ..softmax import SoftmaxFusionType | ||
| from ..sharding import num_of_devices | ||
| from ..sharding import get_sharding_map_logic_axis_to_mesh_axis | ||
|
|
@@ -843,14 +844,30 @@ def __call__( | |
| use_fused_attn = enable_fused_attn and has_fused_attn_kernel | ||
|
|
||
| if enable_fused_attn and not has_fused_attn_kernel: | ||
| warnings.warn( | ||
| "Fused attention is not enabled because there is no available kernel.\n" | ||
| "Fall back to the unfused attention.\n" | ||
| "Please try to update the cuDNN and TE to the latest version.\n" | ||
| f"{qkv_layout=}\n{attn_bias_type=}\n{attn_mask_type=}\n" | ||
| f"{self.attention_dropout=}\n{self.num_attention_heads=}\n{self.window_size=}\n" | ||
| f"{self.num_gqa_groups=}\n{seqlen_q=}\n{seqlen_kv=}\n{head_dim_qk=}\n{head_dim_v=}\n" | ||
| ) | ||
| if ( | ||
| input_dtype in (jnp.float16, jnp.bfloat16) | ||
| and not deterministic | ||
| and qkv_layout.is_thd() | ||
| and softmax_type == AttnSoftmaxType.LEARNABLE_SOFTMAX | ||
| and get_cudnn_version() < (9, 26, 0) | ||
| and head_dim_v not in (64, 128, 256) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like we should disable all head dims for cuDNN < 9.26 based on our Slack conversation? Also, do we need the "not deterministic" here? |
||
| ): | ||
| warnings.warn( | ||
| "Disabling fused attention due to a known cuDNN issue with THD learnable " | ||
| f"softmax backward and head_dim_v = {head_dim_v}. Upgrade to cuDNN 9.26 or " | ||
| "later to use fused attention for this configuration. Falling back to " | ||
| "unfused attention." | ||
| ) | ||
| else: | ||
| warnings.warn( | ||
| "Fused attention is not enabled because there is no available kernel.\n" | ||
| "Fall back to the unfused attention.\n" | ||
| "Please try to update the cuDNN and TE to the latest version.\n" | ||
| f"{qkv_layout=}\n{attn_bias_type=}\n{attn_mask_type=}\n" | ||
| f"{self.attention_dropout=}\n{self.num_attention_heads=}\n{self.window_size=}\n" | ||
| f"{self.num_gqa_groups=}\n{seqlen_q=}\n{seqlen_kv=}\n{head_dim_qk=}\n" | ||
| f"{head_dim_v=}\n" | ||
| ) | ||
|
|
||
| dropout_rng = None | ||
| if not deterministic and self.attention_dropout > 0.0: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1491,7 +1491,22 @@ def _is_fa3_supported(num_heads, num_gqa_groups, head_dim_qk, head_dim_v, qkv_dt | |
| deterministic, | ||
| ) | ||
| if fused_attention_backend == FusedAttnBackend.No_Backend.value: | ||
| logger.debug("Disabling FusedAttention as no backend supports the provided input") | ||
| if ( | ||
| q_type in (TE_DType[torch.float16], TE_DType[torch.bfloat16]) | ||
| and is_training | ||
| and qkv_format == "thd" | ||
| and softmax_type == "learnable" | ||
| and cudnn_version < (9, 26, 0) | ||
| and head_dim_v not in (64, 128, 256) | ||
| ): | ||
| logger.warning( | ||
| "Disabling FusedAttention due to a known cuDNN issue with THD learnable " | ||
| "softmax backward and head_dim_v = %s. Upgrade to cuDNN 9.26 or later to " | ||
| "use FusedAttention for this configuration.", | ||
| head_dim_v, | ||
| ) | ||
| else: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need this if we already have the logic in common? I know Jax is a bit different and might still need it? |
||
| logger.debug("Disabling FusedAttention as no backend supports the provided input") | ||
| use_fused_attention = False | ||
| fused_attention_backend = None | ||
| elif ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just and FYI: I'm adding it in PR 2964 (last commit) as well.