From c00f45e29650f518c4f8f42be8b0fd9e8fb63130 Mon Sep 17 00:00:00 2001 From: Rahul <137375203+therahul-yo@users.noreply.github.com> Date: Tue, 19 May 2026 19:32:08 +0530 Subject: [PATCH] =?UTF-8?q?fix(sensing):=20finish=20#611=20NaN-panic=20aud?= =?UTF-8?q?it=20=E2=80=94=207=20more=20sites=20missed=20by=20#613=20(#624)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #613 fixed adaptive_classifier.rs:94 (the IQR sort) and called the audit done, but the grep used `partial_cmp(b).unwrap()` as a literal and missed seven additional production sites that use comparator variants: adaptive_classifier.rs:205 AdaptiveModel::classify() argmax over softmax probs — same per-frame hot path as #611. NaN flows through normalise → logits → softmax and still reaches this site even after the IQR fix. adaptive_classifier.rs:480 train() argmax (training accuracy loop) adaptive_classifier.rs:500 train() per-class argmax main.rs:2446, 2449 count_persons_mincut variance source/sink select csi.rs:602, 605 count_persons_mincut variance source/sink select (duplicate of main.rs logic in csi.rs) For the variance-select sites, note that the *outer* `unwrap_or((0, &0))` only catches an empty iterator — it cannot rescue a panic raised inside the comparator. A single NaN in `variances[]` still aborts the process. Same fix as #613: swap `.unwrap()` for `.unwrap_or(std::cmp::Ordering::Equal)` inside the comparator closure. Pure behavioural change, no API surface. Re-audit of the remaining `partial_cmp(...).unwrap()` matches in v2/: they are all inside `#[cfg(test)]` / `#[test]` blocks (spectrogram.rs:269, depth.rs:234, connectivity.rs:477, vital_signs.rs:737) where inputs are controlled and panic-on-NaN is acceptable. --- CHANGELOG.md | 18 ++++++++++++++++++ .../src/adaptive_classifier.rs | 10 ++++++---- .../wifi-densepose-sensing-server/src/csi.rs | 6 ++++-- .../wifi-densepose-sensing-server/src/main.rs | 9 ++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44174dd3..20d3a897 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 process. Swapped for `unwrap_or(Ordering::Equal)`, matching the pattern the same file already used at lines 149-150 and 155. Per-frame hot path; this was a real production crash vector. +- **Completed the #611 NaN-panic audit across the sensing-server crate** (follow-up + to #613). The original audit grepped for the literal `partial_cmp(b).unwrap()` + and missed seven additional production sites that use comparator variants + (`partial_cmp(b.1).unwrap()`, `partial_cmp(&variances[b]).unwrap()`). All share + the same crash class — a single `NaN` in CSI-derived state panics the whole + sensing-server. Fixed: + - `adaptive_classifier.rs:205` — `AdaptiveModel::classify()` argmax over softmax + probs. **Same per-frame hot path as #611**; NaN flows through normalise → + logits → softmax and still reaches this site even after the #613 IQR fix. + - `adaptive_classifier.rs:480, 500` — training-loop argmax in `train()` + (training/per-class accuracy reporting). + - `main.rs:2446, 2449` and `csi.rs:602, 605` — variance-based source/sink + selection in `count_persons_mincut`. The outer `unwrap_or((0, &0))` only + catches an empty iterator; it cannot rescue a comparator panic. + + Remaining `partial_cmp(...).unwrap()` sites in the workspace are all inside + `#[cfg(test)]` / `#[test]` blocks (`spectrogram.rs:269`, `depth.rs:234`, + `connectivity.rs:477`, `vital_signs.rs:737`) where inputs are controlled. - **`ui/utils/pose-renderer.js` no longer divides by zero** when two render frames land in the same `performance.now()` tick (issue #519 Bug 2). `deltaTime` is now `Math.max(currentTime - lastFrameTime, 1)` before the `1000 / deltaTime` division, capping displayed FPS at 1000 — far above any real render rate, but finite so the EMA `averageFps = averageFps * 0.9 + fps * 0.1` no longer poisons itself to `Infinity` on a single zero-dt tick. ### Removed diff --git a/v2/crates/wifi-densepose-sensing-server/src/adaptive_classifier.rs b/v2/crates/wifi-densepose-sensing-server/src/adaptive_classifier.rs index cc652f43..0c6f804b 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/adaptive_classifier.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/adaptive_classifier.rs @@ -200,9 +200,11 @@ impl AdaptiveModel { probs[c] = ((logits[c] - max_logit).exp()) / exp_sum; } - // Pick argmax. + // Pick argmax. Same NaN-panic class as #611: if any raw_feature is NaN + // it propagates through normalize → logits → softmax, then partial_cmp + // returns None and unwrap() panics the sensing server on every frame. let (best_c, best_p) = probs.iter().enumerate() - .max_by(|a, b| a.1.partial_cmp(b.1).unwrap()) + .max_by(|a, b| a.1.partial_cmp(b.1).unwrap_or(std::cmp::Ordering::Equal)) .unwrap(); let label = if best_c < self.class_names.len() { self.class_names[best_c].clone() @@ -477,7 +479,7 @@ pub fn train_from_recordings(recordings_dir: &Path) -> Result Result>) -> } } + // partial_cmp returns None on NaN; the outer unwrap_or only catches an + // empty iterator, not a comparator panic. Same NaN-panic class as #611. let (max_var_idx, _) = active.iter().enumerate() - .max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap()) + .max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal)) .unwrap_or((0, &0)); let (min_var_idx, _) = active.iter().enumerate() - .min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap()) + .min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal)) .unwrap_or((0, &0)); if max_var_idx == min_var_idx { return 1; } diff --git a/v2/crates/wifi-densepose-sensing-server/src/main.rs b/v2/crates/wifi-densepose-sensing-server/src/main.rs index 5c763445..b68ee4b9 100644 --- a/v2/crates/wifi-densepose-sensing-server/src/main.rs +++ b/v2/crates/wifi-densepose-sensing-server/src/main.rs @@ -2559,12 +2559,15 @@ fn estimate_persons_from_correlation(frame_history: &VecDeque>) -> usiz } } - // Source → highest-variance subcarrier, Sink → lowest-variance + // Source → highest-variance subcarrier, Sink → lowest-variance. + // partial_cmp returns None on NaN; the outer unwrap_or only catches an + // empty iterator, not a comparator panic. Same NaN-panic class as #611 + // — a single NaN variance frame would kill the sensing-server process. let (max_var_idx, _) = active.iter().enumerate() - .max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap()) + .max_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal)) .unwrap_or((0, &0)); let (min_var_idx, _) = active.iter().enumerate() - .min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap()) + .min_by(|(_, &a), (_, &b)| variances[a].partial_cmp(&variances[b]).unwrap_or(std::cmp::Ordering::Equal)) .unwrap_or((0, &0)); if max_var_idx == min_var_idx {