Skip to content

spearman_rank_corr panics on NaN input (partial_cmp unwrap in compute_ranks) #351

Description

@krishna3554

Summary

compute_ranks inside spearman_rank_corr sorts with partial_cmp(...).unwrap(), which panics when either input slice contains NaN. Since spearman_rank_corr accepts arbitrary &[f64] financial series — where NaN is a routine representation of missing bars/indicators — any NaN in the input crashes the caller instead of returning None like the sibling pearson_corr handles gracefully.

Location

indexed_data.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap());

Problem

f64::partial_cmp returns None whenever either operand is NaN (NaN compares unordered to everything, including itself), so .unwrap() panics with "called Option::unwrap() on a None value" on first NaN encountered by the comparator.

Context that makes this likely to be hit in practice:

  1. The crate's own conventions elsewhere treat missing data as NaN (typical for pandas→Rust ported pipelines feeding czsc-python).
  2. The public contract documented on these functions only mentions empty/mismatched-length handling ("当数据为空或长度不一致时返回 None") — a caller has no reason to pre-filter NaN.
  3. pearson_corr, called at the end of spearman_rank_corr, tolerates NaN inputs (it would return Some(NaN) rather than crash) — so the two correlation helpers have inconsistent failure behavior for identical inputs.

Secondary note: even if the panic were avoided, NaN values would fall through the tie-grouping loop as self-unequal singletons and receive sequential ranks, silently producing a meaningless coefficient. Returning None (or an error) is the semantically correct outcome.

Trigger / Reproduction

Static analysis finding — not confirmed by execution; derived from the comparator at master (701e480a):

let x = [1.0, 2.0, f64::NAN];
let y = [1.0, 2.0, 3.0];
spearman_rank_corr(&x, &y); // panics: unwrap on None
// pearson_corr(&x, &y);     // returns Some(...) without panicking

Expected Behavior

Either return None when any element is non-finite (mirroring the documented None-cases), or use a total-order comparator such as total_cmp (stable since Rust 1.62) so NaN sorts deterministically — though returning None better matches the function's existing contract.

Actual Behavior

Panic propagates out of spearman_rank_corr into the calling signal/trader pipeline.

Impact

A single NaN bar in a series passed to Spearman-based signals aborts the whole computation thread/process instead of degrading to "no correlation", a meaningful robustness gap for a market-data library where NaN is the standard sentinel for absent data.

Suggested Direction

Replace the comparator with a.0.total_cmp(&b.0) if NaN should be ranked deterministically, or add an early if x.iter().any(|v| v.is_nan()) || y.iter().any(|v| v.is_nan()) { return None; } guard in spearman_rank_corr. A unit test with NaN input would lock the chosen behavior in.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions