[JakeStevens]

Fixes the native-ggml failure on main since #23904 (test_hf_whisper.py::WhisperNativeExportTest::test_ggml_runner_matches_eager):

error: ggml: row indices must come from a graph input, through integer casts, views or repeats, or an integer arange

#24422 bumped transformers from 5.0.0rc1 to 5.11.0. In 5.11, WhisperDecoder.forward ignores cache_position and computes position_ids = arange(T) + past_key_values.get_seq_length(), so the position-embedding aten.index.Tensor takes its indices from an aten.add, which GraphBuilder::check_row_indices rejects. The exporter already routed the self-attention cache writes through cache_position (PositionedStaticLayer). This change does the same for the position embeddings by passing position_ids=cache_position.unsqueeze(0).

Test plan: exported a small random-weight Whisper decoder with transformers 5.11.0 and 5.0.0rc1 and traced every index op's indices. Before this change, under 5.11, the position-embedding gather traced to repeat <- unsqueeze <- add. With it, the gather traces to unsqueeze <- cache_position on both versions, and the index_copy cache writes are unchanged. The native-ggml job on this PR runs the end-to-end runner test.

This PR was authored with Claude Code.

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24850

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (7 Unrelated Failures)

As of commit 002b04d with merge base 2dacff2 ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

• Test QNN Backend / test-qnn / test-backend-linux (qnn, operators) / linux-job (gh) (similar failure)
  curl: (22) The requested URL returned error:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / unittest / linux / linux-job (gh) (trunk failure)
  [ FAILED ] OpLeTensorOutTest.BroadcastBoolInputs
• pull / unittest / macos / macos-job (gh) (trunk failure)
  [ FAILED ] OpLeTensorOutTest.BroadcastBoolInputs
• pull / unittest / windows / windows-job (gh) (trunk failure)
  [ FAILED ] OpLeTensorOutTest.BroadcastBoolInputs
• Test QNN Backend / test-qnn / test-backend-linux (qnn, models) / linux-job (gh) (trunk failure)
  curl: (22) The requested URL returned error:
• Test QNN Windows MSVC build / build-qnn-windows-arm64 (gh) (trunk failure)
• Test QNN Windows MSVC build / build-qnn-windows-x64 / windows-job (gh) (trunk failure)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[JakeStevens]

@claude review effort:max

[claude]

Claude finished @JakeStevens's task in 2m 58s —— View job

---

The fix is correct for the ggml row-index check, and I found nothing that blocks merging; there are two minor points (a misleading comment and a missing CPU-only regression test) and one low-confidence question about the sibling LLM exporter.

Detailed review

PR Review: #24850

Summary

This PR passes position_ids=cache_position.unsqueeze(0) to the HF Whisper decoder. Under transformers 5.11, the position-embedding gather then reads its indices from a graph input instead of from arange + get_seq_length(). Nothing blocks the change. Three independent reviewers looked at it and all raised the comment and test-coverage issues below.

Not verified: none of the reviewers could read the installed transformers source, so the claims about WhisperDecoder.forward and WhisperPositionalEmbedding in 5.0.0rc1 and 5.11.0 rest on the PR's trace and on CI. On the ggml side, check_row_indices in GgmlGraphBuilder.cpp was confirmed to walk back through unsqueeze to a placeholder, so the new graph shape is accepted.

Code Quality

• The edited comment overstates the version range and is far from the line it explains (backends/native/examples/audio/export_hf_whisper.py:203-204).
  • It says that "transformers 5" takes positions from the cache's length "both for cache writes and position embeddings". By the PR's own description, only 5.11 ignores cache_position for the embeddings; 5.0.0rc1 already honoured it.
  • It sits above the cumulative_length/positions loop, which only handles cache writes through PositionedStaticLayer. The position_ids= argument it now partly explains is at line 223, 20 lines further down, with no note of its own.
  • Suggested fix: restore the original one-line comment above the loop. At line 223, either add a short note (for example, "5.11 derives embedding positions from get_seq_length()") or none at all.

Testing

• No CPU-only test catches this regression (backends/native/test/test_hf_whisper.py).
  • The only test that fails is test_ggml_runner_matches_eager, and it is skipped unless NATIVE_WHISPER_RUNNER is set, so it runs only in the native-ggml job.
  • test_all_methods_wholly_delegate only checks partitioning, and the index op still delegated before this fix.
  • test_exported_programs_match_eager compares the exported program against the same wrapper, so a position mistake would show up on both sides and the test would still pass.
  • This is why Update transformers version from 5.0.0rc1 to 5.11.0 #24422 broke main without any general pytest failure. A cheap guard would be a graph assertion in the general run (pytest.ini already includes backends/native/test): every aten.index.Tensor index in the exported decoder should trace back through view-like ops to a placeholder.
  • Optional for this PR, but it would stop the next transformers bump from bringing the problem back unnoticed.

Code Quality (follow-up question, low confidence)

• backends/native/examples/hf_llm/export_hf_llm.py:184-196 uses the same cumulative_length workaround, with the same comment, and does not pass position_ids.
  • Under 5.11, RoPE position_ids there would also come from arange + get_seq_length().
  • LowerHFAttention.cpp:417 matches with is_unsqueezed(graph, q_rope->position_ids, k_write->cache_position, 0), so Vulkan attention fusion might silently fail to match.
  • Models with learned absolute position embeddings would hit the same check_row_indices rejection.
  • This was not confirmed by tracing an LLM export. Worth a quick check, or at least a note that it's out of scope.

Recommendation

Approve: the fix is minimal and targeted. Fixing the comment is a nit. The CPU-only guard test and the LLM-exporter check can be done as follow-ups.