🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure

As of commit 85f0845 with merge base 124b677 ():

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

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.

@claude review and check for any areas or opportunities for modularization

Claude finished @JulianCloudNTH's task in 1m 28s —— View job

Review: dynamic-shape integration test

• Read changed files
• Review for correctness & best practices
• Identify modularization opportunities

Solid end-to-end test — building one graph at S=MAXS and replaying smaller live shapes against a torch golden is exactly the right oracle for the allocate-at-max + per-op-resize engine, and selecting multi-output tensors by full shape (not numel) is the correct call. The graceful SKIP paths (no WebGPU device, SDPA op-coverage PENDING) keep the suite green where appropriate. Below are the modularization opportunities you asked about, plus a few minor notes.

Modularization — Python (test_dynamic_shape_export.py)

1. The lower → assert-delegated → write-buffer block is copy-pasted 5×. _export (lines 97-108) already does this, but _export_dynamic_linear, _export_dynamic_sdpa, _export_dynamic_embedding, and _export_dynamic_rope each re-implement the identical to_edge_transform_and_lower(...).to_executorch() + any(d.id == "VulkanBackend" ...) + open().write(et.buffer) sequence (e.g. lines 210-218, 253-261, 292-299, 335-343). Since the only thing that differs between callers is how the ExportedProgram is built, split _export so everyone shares the tail:

def _lower_and_write(ep, path: str) -> None:
    et = to_edge_transform_and_lower(ep, partitioner=[VulkanPartitioner()]).to_executorch()
    assert any(
        d.id == "VulkanBackend"
        for plan in et.executorch_program.execution_plan
        for d in plan.delegates
    ), f"Expected VulkanBackend delegate in {path}"
    with open(path, "wb") as f:
        f.write(et.buffer)
    print(f"Exported {path}")

_export then becomes torch.export.export(...) + _lower_and_write(ep, path), and the four specialized exporters drop ~6 duplicated lines each. Fix this →

2. The tensor → little-endian f32 → file incantation appears ~10×. t.detach().cpu().numpy().astype("<f4").tofile(os.path.join(out_dir, ...)) is repeated in _write_goldens, _export_dynamic_linear, _export_dynamic_sdpa, _export_dynamic_embedding, and _export_dynamic_rope. A one-liner _write_f32(t, path) (and a sibling _write_i64 for the embedding idx at line 307) would centralize the dtype contract that the native side depends on, so a future dtype change is one edit instead of ten.

3. SDPA and RoPE golden loops are structurally identical (lines 266-270, 347-351): iterate [(name, tensor), ...] and dump each to {prefix}.S{s}.{name}.bin. Once _write_f32 exists, both collapse to a shared _write_named(prefix, s, pairs, out_dir).

Modularization — C++ (test_dynamic_shape.cpp)

4. The four op-specific checkers share a fixed skeleton. check_linear, check_sdpa, check_embedding, and check_rope each repeat: construct Module, load_forward() with FAIL/ok=false handling, read input bins, make_tensor_ptr, forward, result .ok()/isTensor guards, slice to numel, max_err, and the printf(... PASS/FAIL); ok = ok && pass tail. The per-op delta is really just (a) input tensor construction, (b) output selection, and (c) tolerance. Worth extracting the invariant pieces:

• forward_and_check(got, golden, label, s, tol, ok) — the max_err + PASS/FAIL print + ok &= pass tail (currently duplicated in check_s, check_linear, check_embedding, check_rope, check_sdpa).
• find_output_by_shape(result, predicate) — the shape-matching output-selection loop is written twice (SDPA lines 207-217, RoPE lines 309-321) with the same structure.

5. The main()-level "load fresh module per S and check_s" loop is repeated 4× for cases A/B, F, G, H (lines 360-369, 398-407, 410-419, 422-431) — identical except the .pte name and the S list. A helper:

void run_dyn_cases(const std::string& dir, const std::string& prefix,
                   std::initializer_list<int> ss, bool& ok) {
  for (int s : ss) {
    Module m(dir + "/" + prefix + ".pte");
    if (m.load_forward() != Error::Ok) { printf("  FAIL load %s.pte\n", prefix.c_str()); ok = false; return; }
    check_s(m, dir, prefix, s, ok);
  }
}

turns each case into a single call. Fix this →

6. Templatize the binary reader. read_bin only handles float, so check_embedding (lines 246-255) hand-rolls the int64 read. A read_bin<T>(path) would remove that special case and keep one file-reading code path.

Minor notes (non-blocking)

• Magic tolerances. 1e-3f, 5e-3f, 2e-3f are inlined at each call site (lines 106, 155, 230, 270, 331). Named constants (kTolRms, kTolQuant, kTolSdpa) would document intent and pair naturally with the tol parameter in suggestion Add unlifting pass under private config #4.
• Header comment lists A–F but cases extend to L (lines 9-21). The block comment stops at case F while the suite runs through L — worth completing or trimming so it doesn't drift.
• max_err returns 1e30f as a size-mismatch sentinel (line 57). It works because every comparison is < tol, but an explicit got.empty() || size mismatch → FAIL at the call site reads more clearly than relying on a magic large float comparing false.

None of these are correctness blockers — the test logic itself looks right. The Python _lower_and_write extraction (#1) is the highest-value change; the C++ helpers (#4/#5) are next.