fix(ml): migrate test uploads from clone_htod to mapped-pinned
Per feedback_no_htod_htoh_only_mapped_pinned: mapped-pinned only for
CPU↔GPU; tests not exempt. Previous commit ab64c0412 was a shallow
deprecated-API swap (memcpy_stod -> clone_htod) — both still HTOD
copies. This commit replaces all 5 uploads and the 1 readback in
test_eval_action_select_thompson_picks_proportionally with the
production mapped-pinned + memcpy_dtod_async pattern (mirrors lines
433-448 of the same file).
Closure 'upload_pinned' DRYs the 5 upload sites; readback uses
MappedI32Buffer + DtoD into staging.host_slice_mut().to_vec().
Zero HTOD/HTOH copies remain in this file. Documented in
dqn-wire-up-audit.md.
This commit is contained in:
@@ -1347,10 +1347,38 @@ mod tests {
|
||||
let batch: usize = 8192;
|
||||
let n_atoms: i32 = 21;
|
||||
|
||||
// Per `feedback_no_htod_htoh_only_mapped_pinned`: every CPU↔GPU
|
||||
// transfer uses mapped-pinned staging + DtoD, never htod copies.
|
||||
// Closure factors the 5 host buffers below — host writes the slice
|
||||
// into pinned memory and the kernel reads via DtoD into a device
|
||||
// CudaSlice. Identical to the production pattern at lines 433-448.
|
||||
let upload_pinned = |host: &[f32]| -> cudarc::driver::CudaSlice<f32> {
|
||||
let n = host.len();
|
||||
let staging = unsafe { mapped_pinned::MappedF32Buffer::new(n) }
|
||||
.expect("mapped-pinned upload staging alloc");
|
||||
staging.write_from_slice(host);
|
||||
let mut dst = stream.alloc_zeros::<f32>(n).expect("device dst alloc");
|
||||
if n > 0 {
|
||||
let nbytes = n * std::mem::size_of::<f32>();
|
||||
unsafe {
|
||||
let (dst_ptr, _g) = dst.device_ptr_mut(&stream);
|
||||
cudarc::driver::result::memcpy_dtod_async(
|
||||
dst_ptr,
|
||||
staging.dev_ptr,
|
||||
nbytes,
|
||||
stream.cu_stream(),
|
||||
)
|
||||
.expect("mapped-pinned upload DtoD");
|
||||
}
|
||||
stream.synchronize().expect("mapped-pinned upload sync");
|
||||
}
|
||||
dst
|
||||
};
|
||||
|
||||
// q_values: uniform Q (zero) so direction is decided purely by C51
|
||||
// logits below; magnitude/order/urgency default to Boltzmann-uniform.
|
||||
let q_host = vec![0.0_f32; batch * q_stride];
|
||||
let q_buf = stream.clone_htod(&q_host).expect("upload q_values");
|
||||
let q_buf = upload_pinned(&q_host);
|
||||
|
||||
// C51 logits: per-direction peaked at atom corresponding to target v.
|
||||
// Linear support v_min=-1 v_max=+1 delta_z=0.1 → atom_v = -1 + a*0.1.
|
||||
@@ -1370,7 +1398,7 @@ mod tests {
|
||||
b_logits_host[base + peak_a] = 5.0_f32;
|
||||
}
|
||||
}
|
||||
let b_logits_buf = stream.clone_htod(&b_logits_host).expect("upload logits");
|
||||
let b_logits_buf = upload_pinned(&b_logits_host);
|
||||
|
||||
// per_sample_support: [batch, b0, 3] stride-12 — every sample/direction
|
||||
// gets the same linear support [v_min, v_max, delta_z].
|
||||
@@ -1383,7 +1411,7 @@ mod tests {
|
||||
support_host[base + 2] = delta_z;
|
||||
}
|
||||
}
|
||||
let support_buf = stream.clone_htod(&support_host).expect("upload support");
|
||||
let support_buf = upload_pinned(&support_host);
|
||||
|
||||
// SP17 Commit C (2026-05-08): v_logits_dir [batch, n_atoms] — V-head
|
||||
// logits row required by the Thompson direction selector. Required
|
||||
@@ -1397,7 +1425,7 @@ mod tests {
|
||||
// pre-SP17). Without this buffer the kernel reads stale stack
|
||||
// memory for v_logits_dir → SIGSEGV.
|
||||
let v_logits_host = vec![0.0_f32; batch * (n_atoms as usize)];
|
||||
let v_logits_buf = stream.clone_htod(&v_logits_host).expect("upload v_logits");
|
||||
let v_logits_buf = upload_pinned(&v_logits_host);
|
||||
|
||||
// SP10: ISV buffer with EVAL_THOMPSON_TEMP_INDEX set to 1.0
|
||||
// (pure Thompson). All other slots zero — kernel only reads
|
||||
@@ -1405,7 +1433,7 @@ mod tests {
|
||||
// block reads ISV[16]/[21] with cold-start fallback.
|
||||
let mut isv_host = vec![0.0_f32; ISV_TOTAL_DIM];
|
||||
isv_host[EVAL_THOMPSON_TEMP_INDEX] = 1.0;
|
||||
let isv_buf = stream.clone_htod(&isv_host).expect("upload isv");
|
||||
let isv_buf = upload_pinned(&isv_host);
|
||||
|
||||
let mut actions_buf = stream.alloc_zeros::<i32>(batch).expect("alloc actions");
|
||||
let mut q_gaps_buf = stream.alloc_zeros::<f32>(batch).expect("alloc q_gaps");
|
||||
@@ -1463,7 +1491,27 @@ mod tests {
|
||||
}
|
||||
stream.synchronize().expect("sync");
|
||||
|
||||
let host_actions = stream.clone_dtoh(&actions_buf).expect("readback actions");
|
||||
// Readback via mapped-pinned (per `feedback_no_htod_htoh_only_mapped_pinned`):
|
||||
// DtoD copy from device actions_buf into pinned staging, then read the
|
||||
// host_slice — same coherence pattern as production at lines 433-448.
|
||||
let host_actions: Vec<i32> = {
|
||||
let n = actions_buf.len();
|
||||
let mut staging = unsafe { mapped_pinned::MappedI32Buffer::new(n) }
|
||||
.expect("mapped-pinned readback staging alloc");
|
||||
let nbytes = n * std::mem::size_of::<i32>();
|
||||
unsafe {
|
||||
let (src_ptr, _g) = actions_buf.device_ptr(&stream);
|
||||
cudarc::driver::result::memcpy_dtod_async(
|
||||
staging.dev_ptr,
|
||||
src_ptr,
|
||||
nbytes,
|
||||
stream.cu_stream(),
|
||||
)
|
||||
.expect("mapped-pinned readback DtoD");
|
||||
}
|
||||
stream.synchronize().expect("mapped-pinned readback sync");
|
||||
staging.host_slice_mut().to_vec()
|
||||
};
|
||||
let mut dir_hist = [0_u32; 4];
|
||||
for &a in &host_actions {
|
||||
let dir = (a / (b1 * b2 * b3)) as usize;
|
||||
|
||||
@@ -18792,3 +18792,26 @@ names. Per `feedback_no_legacy_aliases`: rename call sites directly, no
|
||||
deprecated wrappers retained.
|
||||
|
||||
**Wire-up impact:** None (API surface unchanged; FFI semantics identical).
|
||||
|
||||
## 2026-05-22 — fix(ml): migrate test uploads from clone_htod to mapped-pinned
|
||||
|
||||
**Branch:** `ml-alpha-phase-a`. **Trigger:** prior commit (`ab64c0412`) had
|
||||
swapped deprecated `memcpy_stod`/`memcpy_dtov` for `clone_htod`/`clone_dtoh`
|
||||
— but both are HTOD/HTOH copies. Per `feedback_no_htod_htoh_only_mapped_pinned`
|
||||
(memory pearl), mapped-pinned is the only permitted CPU↔GPU path, with
|
||||
tests not exempt.
|
||||
|
||||
**Change:** `crates/ml/src/cuda_pipeline/mod.rs` —
|
||||
`test_eval_action_select_thompson_picks_proportionally`:
|
||||
- Introduced `upload_pinned` closure that allocates a `MappedF32Buffer`,
|
||||
`write_from_slice`s host data into the pinned page, allocates a device
|
||||
destination via `stream.alloc_zeros`, and `memcpy_dtod_async` copies
|
||||
staging.dev_ptr → dst with a stream sync. This is the same pattern used
|
||||
at lines 433-448 (the production data-loader path).
|
||||
- Replaced 5 `clone_htod` calls with `upload_pinned(&host)`.
|
||||
- Replaced the 1 `clone_dtoh` readback with a `MappedI32Buffer` + DtoD
|
||||
copy from `actions_buf` to `staging.dev_ptr`, then
|
||||
`staging.host_slice_mut().to_vec()`.
|
||||
|
||||
**Wire-up impact:** None (test still produces the same direction histogram
|
||||
assertions; only the FFI path changed).
|
||||
|
||||
Reference in New Issue
Block a user