Skip to content

fix: Optional segment position annotations in mask helpers - #3369

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/test-fused-attn-optional-segment-position-annotations
Open

fix: Optional segment position annotations in mask helpers#3369
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/test-fused-attn-optional-segment-position-annotations

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

This PR addresses the following issue in tests/jax/test_fused_attn.py: Optional segment position annotations in mask helpers.

Changes

  • tests/jax/test_fused_attn.py: Optional segment position annotations in mask helpers.

Details

--- a/tests/jax/test_fused_attn.py
+++ b/tests/jax/test_fused_attn.py
@@ -1,19 +1,19 @@
-@jax.jit
-def make_causal_mask(
-    segment_ids_q: ArrayLike,
-    segment_ids_kv: ArrayLike,
-    segment_pos_q: ArrayLike = None,
-    segment_pos_kv: ArrayLike = None,
-) -> Array:
-
-...
-
-@partial(jax.jit, static_argnums=(4, 5))
-def make_mask(
-    segment_ids_q: ArrayLike,
-    segment_ids_kv: ArrayLike,
-    segment_pos_q: ArrayLike,
-    segment_pos_kv: ArrayLike,
-    attn_mask_type: AttnMaskType,
-    window_size: Optional[Tuple[int, int]] = None,
-) -> Array:
+@jax.jit
+def make_causal_mask(
+    segment_ids_q: ArrayLike,
+    segment_ids_kv: ArrayLike,
+    segment_pos_q: Optional[ArrayLike] = None,
+    segment_pos_kv: Optional[ArrayLike] = None,
+) -> Array:
+
+...
+
+@partial(jax.jit, static_argnums=(4, 5))
+def make_mask(
+    segment_ids_q: ArrayLike,
+    segment_ids_kv: ArrayLike,
+    segment_pos_q: Optional[ArrayLike],
+    segment_pos_kv: Optional[ArrayLike],
+    attn_mask_type: AttnMaskType,
+    window_size: Optional[Tuple[int, int]] = None,
+) -> Array:

Tests

  • tests/jax/test_fused_attn.py
--- a/tests/jax/test_fused_attn.py
+++ b/tests/jax/test_fused_attn.py
@@ -180,6 +180,28 @@
     mask = jnp.logical_not(inv_mask)
     return mask
 
+
+def test_mask_helpers_handle_none_segment_pos():
+    """Verify make_causal_mask and make_mask fall back to arange when segment_pos is None."""
+    seq = 8
+    segment_ids = jnp.array(
+        [[1, 1, 1, 1, 0, 0, 0, 0], [2, 2, 2, 2, 2, 0, 0, 0]], dtype=jnp.int32
+    )
+    segment_pos = jnp.broadcast_to(jnp.arange(seq, dtype=jnp.int32), segment_ids.shape)
+
+    # make_causal_mask uses default None for segment positions.
+    mask_with_pos = make_causal_mask(segment_ids, segment_ids, segment_pos, segment_pos)
+    mask_without_pos = make_causal_mask(segment_ids, segment_ids)
+    assert mask_with_pos.shape == mask_without_pos.shape
+    assert jnp.array_equal(mask_with_pos, mask_without_pos)
+
+    # make_mask accepts explicit None for segment positions.
+    for mask_type in (AttnMaskType.PADDING_MASK, AttnMaskType.CAUSAL_MASK):
+        mask_with_pos = make_mask(
+            segment_ids, segment_ids, segment_pos, segment_pos, mask_type
+        )
+        mask_without_pos = make_mask(segment_ids, segment_ids, None, None, mask_type)
+        assert mask_with_pos.shape == mask_without_pos.shape
+        assert jnp.array_equal(mask_with_pos, mask_without_pos)
+
 
 @jax.jit
 def get_seqlens_and_offsets(segment_ids):

Squashed to single commit for review.

Original PR: #9
@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Aug 13, 2026
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Corrects a typo in the pytest skip reason for unsupported non-zero dropout with bottom-right causal masking.

  • Replaces “dropoouts” with “dropouts.”
  • Removes unnecessary f-string formatting from the static message.

Confidence Score: 5/5

The PR appears safe to merge because it only corrects a static pytest skip message.

The skip condition and control flow are unchanged, and no repository consumer depends on the previous misspelled message.

Important Files Changed

Filename Overview
tests/jax/test_fused_attn.py Corrects a static skip-message typo without changing test execution or runtime behavior.

Reviews (1): Last reviewed commit: "fix: Optional segment position annotatio..." | Re-trigger Greptile

@cyanguwa cyanguwa left a comment

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.

The code change doesn't match the PR title, and is duplicate to PR 3370.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants