cleanup(unwrap): NaN-safe q_gap partial_cmp in snapshot ring — [UNWRAP-002]

Three sites compared f32 q_gap values via partial_cmp().unwrap() — any
NaN q_gap would panic the snapshot swap-out / best-pointer paths on the
training hot path. Replaced with partial_cmp().unwrap_or(Ordering::Equal)
and tightened the outer Option unwrap to expect() with the invariant text
(len >= MAX_SNAPSHOTS guarantees a worst element exists).
This commit is contained in:
jgrusewski
2026-04-20 22:14:26 +02:00
parent ca6a1a7ba6
commit e508475f3c
2 changed files with 12 additions and 5 deletions

View File

@@ -7957,9 +7957,12 @@ impl GpuDqnTrainer {
if ring.snapshots.len() >= crate::cuda_pipeline::q_snapshot::MAX_SNAPSHOTS {
let worst_idx = ring.snapshots.iter()
.enumerate()
.min_by(|(_, a), (_, b)| a.q_gap.partial_cmp(&b.q_gap).unwrap())
.min_by(|(_, a), (_, b)| {
// NaN-safe: NaN q_gaps compare as Equal instead of panicking.
a.q_gap.partial_cmp(&b.q_gap).unwrap_or(std::cmp::Ordering::Equal)
})
.map(|(i, _)| i)
.unwrap();
.expect("MAX_SNAPSHOTS >= 1 and len >= MAX_SNAPSHOTS, so at least one snapshot exists");
ring.snapshots.swap_remove(worst_idx);
}

View File

@@ -107,9 +107,13 @@ impl SnapshotRing {
if self.snapshots.len() >= MAX_SNAPSHOTS {
let worst_idx = self.snapshots.iter()
.enumerate()
.min_by(|(_, a), (_, b)| a.q_gap.partial_cmp(&b.q_gap).unwrap())
.min_by(|(_, a), (_, b)| {
// NaN-safe: treat NaN q_gaps as equal so a snapshot with NaN
// does not panic the swap-out path.
a.q_gap.partial_cmp(&b.q_gap).unwrap_or(std::cmp::Ordering::Equal)
})
.map(|(i, _)| i)
.unwrap();
.expect("MAX_SNAPSHOTS >= 1 and len >= MAX_SNAPSHOTS, so at least one snapshot exists");
self.snapshots.swap_remove(worst_idx);
}
@@ -120,7 +124,7 @@ impl SnapshotRing {
/// Best (highest q_gap) snapshot raw device pointer, if any.
pub fn best_raw_ptr(&self) -> Option<u64> {
self.snapshots.iter()
.max_by(|a, b| a.q_gap.partial_cmp(&b.q_gap).unwrap())
.max_by(|a, b| a.q_gap.partial_cmp(&b.q_gap).unwrap_or(std::cmp::Ordering::Equal))
.map(|s| s.weights.raw_ptr())
}
}