Skip to content

bug: inf_norm silently returns 0.0 for matrices containing NaN #85

@acgetchell

Description

@acgetchell

Summary

Matrix::inf_norm() silently ignores NaN entries because NaN > max_row_sum evaluates to false. A matrix full of NaN values returns inf_norm() == 0.0, which is mathematically wrong.

Current State

pub fn inf_norm(&self) -> f64 {
    let mut max_row_sum: f64 = 0.0;
    for row in &self.rows {
        let row_sum: f64 = row.iter().map(|&x| x.abs()).sum();
        if row_sum > max_row_sum {
            max_row_sum = row_sum;
        }
    }
    max_row_sum
}

When any entry is NaN, x.abs() returns NaN, sum() returns NaN, and NaN > max_row_sum is always false, so the NaN row is skipped.

let m = Matrix::<2>::from_rows([[f64::NAN, 1.0], [2.0, 3.0]]);
m.inf_norm() // Returns 5.0, ignoring the NaN row entirely

let m = Matrix::<2>::from_rows([[f64::NAN, f64::NAN], [f64::NAN, f64::NAN]]);
m.inf_norm() // Returns 0.0

Proposed Changes

Use f64::max (which propagates NaN) instead of a manual comparison:

pub fn inf_norm(&self) -> f64 {
    let mut max_row_sum: f64 = 0.0;
    for row in &self.rows {
        let row_sum: f64 = row.iter().map(|&x| x.abs()).sum();
        max_row_sum = max_row_sum.max(row_sum);
    }
    max_row_sum
}

Wait — f64::max returns the other argument when one is NaN (IEEE 754 maxNum). So this still wouldn't propagate NaN.

The correct fix is to use f64::maximum (Rust 1.83+, IEEE 754-2019 maximum which propagates NaN):

max_row_sum = f64::maximum(max_row_sum, row_sum);

Or, return NaN explicitly:

if row_sum.is_nan() {
    return f64::NAN;
}

Benefits

  • Correct mathematical behavior: inf_norm of a matrix with NaN entries should be NaN
  • Prevents downstream code from treating a NaN-containing matrix as having a well-defined norm
  • inf_norm is currently only used internally by debug_assert_symmetric, but it's public API and could be used independently

Implementation Notes

  • f64::maximum is available since Rust 1.83 (well within MSRV 1.94)
  • This is a behavior change but arguably a bugfix, not a breaking change
  • The debug_assert_symmetric caller uses inf_norm for scaling — a NaN result there would cause the debug assert to fire, which is the correct behavior

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingrustPull requests that update rust codevalidation

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions