Skip to content

Commit 44cc0f7

Browse files
committed
fix(zenpredict): argmin_masked_top_k skips NaN scores per documented contract
The consider() closure only rejected a NaN score once the top-K array was full (the count == K branch's score < top[K-1].0 comparison, where NaN < x is always false). While count < K, it inserted UNCONDITIONALLY — the shift-loop's top[i - 1].0 > score never fires for a NaN score (every comparison with NaN is false), so the NaN just landed in the next empty slot instead of being skipped. This violated the function's own documented 'same NaN ... contract as argmin_masked' (NaN cells silently skipped, never picked). Fixed: skip immediately when score.is_nan(), before either branch. Added argmin_top_k_skips_nan_scores: 5 predictions with 2 NaNs (both in the count<K window) must never appear in the top-3 result; an all-NaN input must yield [None, None, None]. Verified the test fails without the fix (reproduced: NaN landed at index 0 instead of being skipped) and passes with it. cargo test -p zenpredict --lib --features advanced: 141 passed.
1 parent 834035b commit 44cc0f7

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

zenpredict/src/argmin.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,13 @@ pub fn argmin_masked_top_k<const K: usize>(
239239
let mut count: usize = 0;
240240

241241
let mut consider = |score: f32, idx: usize| {
242+
// Same NaN contract as `argmin_masked`: a NaN-scoring cell is silently skipped, never
243+
// occupying a top-K slot. Without this, `score.is_nan()` cells (for which every `<`
244+
// comparison is false) would slide into an empty slot below simply because the
245+
// shift-loop's `top[i - 1].0 > score` never fires to displace it.
246+
if score.is_nan() {
247+
return;
248+
}
242249
if count < K {
243250
let mut i = count;
244251
while i > 0 && top[i - 1].0 > score {

zenpredict/src/tests.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,28 @@ fn argmin_top_k_returns_sorted_indices() {
5656
assert_eq!(top, [Some(1), Some(3), Some(0)]);
5757
}
5858

59+
#[test]
60+
#[cfg(feature = "advanced")]
61+
fn argmin_top_k_skips_nan_scores() {
62+
// Same documented NaN contract as `argmin_masked`: a NaN-scoring cell must never occupy a
63+
// top-K slot, even while the array still has room (count < K) — that's exactly the case
64+
// the prior bug missed, since the shift-loop's `top[i - 1].0 > score` never fires to
65+
// displace a NaN sitting in an empty slot.
66+
let pred = [f32::NAN, 3.0f32, 1.0, f32::NAN, 1.5];
67+
let mask = [true; 5];
68+
let m = AllowedMask::new(&mask);
69+
let top = argmin::argmin_masked_top_k::<3>(&pred, &m, ScoreTransform::Identity, None);
70+
assert_eq!(top, [Some(2), Some(4), Some(1)]); // 1.0, 1.5, 3.0 — NaNs never appear
71+
72+
// All-NaN allowed set: every slot stays None (matches argmin_all_nan_returns_none).
73+
let all_nan = [f32::NAN; 4];
74+
let mask4 = [true; 4];
75+
let m4 = AllowedMask::new(&mask4);
76+
let top_all_nan =
77+
argmin::argmin_masked_top_k::<3>(&all_nan, &m4, ScoreTransform::Identity, None);
78+
assert_eq!(top_all_nan, [None, None, None]);
79+
}
80+
5981
#[test]
6082
#[cfg(feature = "advanced")]
6183
fn pick_with_confidence_reports_gap() {

0 commit comments

Comments
 (0)