Fix APIC array reduction replay#1669
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughChangesArray reductions now participate in CPU graph and CPU/CUDA APIC capture, serialization, validation, and replay. The change adds reduction records, capture constraints, native replay dispatch, save/load coverage, and runtime documentation. APIC reduction support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ReductionAPI as array_sum/array_inner
participant APICCapture
participant APICReplay
Caller->>ReductionAPI: invoke reduction
ReductionAPI->>APICCapture: validate and record metadata
APICCapture->>APICReplay: serialize reduction operation
APICReplay->>ReductionAPI: dispatch reduction during replay
APICReplay-->>Caller: write output
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5This PR is safe to merge. The changes are well-scoped to the APIC reduction serialization path, all validation is layered (Python pre-flight, native metadata check, stream validator, replay pointer resolution), and the 8 new test functions cover capture correctness, save/load round-trips, empty-input edge cases, and all documented error paths. The implementation follows the same patterns as the existing scan/sort/BSR extended ops. The CPU path correctly skips execution during capture and re-executes on replay; the CUDA path correctly falls through to the CUB dispatch so kernels reach the capture stream as native graph nodes. Overflow and alignment checks are applied at every layer. All 216 tests passed on CPU and two CUDA devices per the author's validation summary. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PY as Python (utils.py)
participant NAT as Native (reduce.cpp/.cu)
participant APIC as APIC State (apic.cpp/.cu)
participant RT as Replay/Rebuild
Note over PY,RT: APIC Capture Phase (CPU or CUDA)
PY->>PY: _get_apic_capture_for_device()
PY->>PY: "validate (count, strides, out != None)"
PY->>APIC: track_array(input / out)
PY->>NAT: wp_array_sum_float_host/device()
alt CPU host path
NAT->>APIC: apic_capture_reduction_host()
APIC->>APIC: apic_reduction_metadata_valid()
APIC->>APIC: "apic_resolve_host_ptr() -> region_id + offset"
APIC->>APIC: apic_record_reduction()
NAT-->>PY: return (skip actual compute)
else CUDA device path
NAT->>APIC: apic_capture_reduction_device()
APIC->>APIC: "apic_resolve_live_ptr() -> region_id + offset"
APIC->>APIC: apic_record_reduction()
NAT->>NAT: "array_sum_device_dispatch() (CUB -> captured by stream)"
end
Note over PY,RT: Replay / Graph Rebuild Phase
RT->>RT: apic_cpu_replay_stream() or apic_replay_ops_into_cuda_capture()
RT->>APIC: read APICReductionRecord
RT->>RT: "resolve region_id+offset -> ptr"
RT->>NAT: wp_array_sum_float_host/device(current ptrs)
NAT->>NAT: Execute reduction on current data
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PY as Python (utils.py)
participant NAT as Native (reduce.cpp/.cu)
participant APIC as APIC State (apic.cpp/.cu)
participant RT as Replay/Rebuild
Note over PY,RT: APIC Capture Phase (CPU or CUDA)
PY->>PY: _get_apic_capture_for_device()
PY->>PY: "validate (count, strides, out != None)"
PY->>APIC: track_array(input / out)
PY->>NAT: wp_array_sum_float_host/device()
alt CPU host path
NAT->>APIC: apic_capture_reduction_host()
APIC->>APIC: apic_reduction_metadata_valid()
APIC->>APIC: "apic_resolve_host_ptr() -> region_id + offset"
APIC->>APIC: apic_record_reduction()
NAT-->>PY: return (skip actual compute)
else CUDA device path
NAT->>APIC: apic_capture_reduction_device()
APIC->>APIC: "apic_resolve_live_ptr() -> region_id + offset"
APIC->>APIC: apic_record_reduction()
NAT->>NAT: "array_sum_device_dispatch() (CUB -> captured by stream)"
end
Note over PY,RT: Replay / Graph Rebuild Phase
RT->>RT: apic_cpu_replay_stream() or apic_replay_ops_into_cuda_capture()
RT->>APIC: read APICReductionRecord
RT->>RT: "resolve region_id+offset -> ptr"
RT->>NAT: wp_array_sum_float_host/device(current ptrs)
NAT->>NAT: Execute reduction on current data
Reviews (4): Last reviewed commit: "Record APIC array reductions (GH-1663)" | Re-trigger Greptile |
9a78233 to
4a8d056
Compare
API Capture graphs recorded kernel launches but omitted the native helpers behind array_sum() and array_inner(). CPU graph replay and serialized CUDA graphs therefore reused capture-time reduction results. Record reduction metadata in APIC streams and rebuild the operations for CPU replay and CUDA graph loading. Validate supported layouts and fail clearly for forms that cannot be replayed safely. Cover public capture/replay and save/load behavior, including axes, empty inputs, and metadata bindings. Document the supported reduction contract. Signed-off-by: Eric Shi <ershi@nvidia.com>
4a8d056 to
3e16144
Compare
Description
Closes #1663.
Record
wp.utils.array_sum()andwp.utils.array_inner()in APIC graphs so CPU graph replay and CPU/CUDA save/load recompute reductions from current inputs. Previously, the native reduction helpers did not append replayable operation metadata, so captured graphs could omit the reduction or retain capture-time results.Changes
Checklist
Unreleasedsection.Validation summary
Built the release native libraries with CUDA 13.0 on driver 13.0, verifying that the C++ and CUDA additions compile and link.
Ran
WARP_CACHE_PATH=/tmp/warp-cache-warp-worktree-4 uv run warp/tests/test_apic.py; all 216 tests passed on CPU and two CUDA devices, including the new array reduction capture, replay, save/load, metadata, and error-handling cases.Ran pre-commit on all changed files; every applicable hook passed.
Built the HTML documentation successfully with no build errors.
Bug fix
Summary by CodeRabbit
wp.utils.array_sum()andwp.utils.array_inner()(CPU and CUDA), with correct save/load persistence and parameter updates.out.