👻 `product` is a cursed test

[akern40]

While on a hunt for tests that are unnecessarily behind feature gates (turns out we have quite a few of them), I ran across the test oper::product that appears to be cursed. Like, adding a no-side-effects assert causes the test to fail, kind of cursed.

I must be missing something. I thought it might be a floating point error problem, but I cannot tell why the addition of an assert statement would create additional floating point error. My current theory is that maybe the assert allows LLVM to optimize the checks differently? But I still don't know why that would invalidate the internal logic of the test.

Here's an equivalent Rust program, running with ndarray 0.16.1:

use approx::assert_abs_diff_eq;
use ndarray::{Array, Ixs, s};

/// Two equivalent methods of doing `linspace`.
/// We show here that the two methods yield _identical_ results.
fn equal_linspace() {
    let a = Array::linspace(0.5, 2., 128)
        .into_shape_with_order((8, 16))
        .unwrap();

    let step = (2. - 0.5) / 127.;
    let a_other = Array::from_iter((0..128).map(|i| 0.5 + step * (i as f32)))
        .into_shape_with_order((8, 16))
        .unwrap();

    // Equality according to both ndarray and Rust slices
    assert_eq!(a, a_other);
    assert_eq!(a.as_slice().unwrap(), a_other.as_slice().unwrap());
}

/// The original `product` test from `oper.rs`.
/// As expected, runs without issue.
fn product() {
    let a = Array::linspace(0.5, 2., 128)
        .into_shape_with_order((8, 16))
        .unwrap();
    assert_abs_diff_eq!(a.fold(1., |acc, &x| acc * x), a.product(), epsilon = 1e-5);

    // test different strides
    let max = 8 as Ixs;
    for i in 1..max {
        for j in 1..max {
            let a1 = a.slice(s![..;i, ..;j]);
            let mut prod = 1.;
            for elt in a1.iter() {
                prod *= *elt;
            }
            assert_abs_diff_eq!(a1.fold(1., |acc, &x| acc * x), prod, epsilon = 1e-5);
            assert_abs_diff_eq!(prod, a1.product(), epsilon = 1e-5);
        }
    }
}

/// The product test, now with an additional, EQUIVALENT array `a_other`.
///
/// 1. We define `a` the same as before.
/// 2. We define `a_other` using a different method, but prior tests show us it's equivalent.
/// 3. We then run the test as before.
/// 4. Adding a simple `assert_eq` between the two arrays causes the test to fail..
fn bad_product() {
    let a = Array::linspace(0.5, 2., 128)
        .into_shape_with_order((8, 16))
        .unwrap();

    let step = (2. - 0.5) / 127.;
    let a_other = Array::from_iter((0..128).map(|i| 0.5 + step * (i as f32)))
        .into_shape_with_order((8, 16))
        .unwrap();

    // UNCOMMENTING THIS LINE WILL CAUSE THE TEST TO FAIL.
    // THIS IS NOT THE LINE THAT CAUSES THE FAILURE.
    // IT WILL FAIL AT THE FIRST assert_abs_diff_eq IN THE LOOP ON i = 1, j = 2.
    assert_eq!(a, a_other);

    assert_abs_diff_eq!(a.fold(1., |acc, &x| acc * x), a.product(), epsilon = 1e-5);

    // test different strides
    let max = 8 as Ixs;
    for i in 1..max {
        for j in 1..max {
            let a1 = a.slice(s![..;i, ..;j]);

            let mut prod = 1.;
            for elt in a1.iter() {
                prod *= *elt;
            }
            eprintln!("{i}, {j}");
            assert_abs_diff_eq!(a1.fold(1., |acc, &x| acc * x), prod, epsilon = 1e-5);
            assert_abs_diff_eq!(prod, a1.product(), epsilon = 1e-5);
        }
    }
}

fn main() {
    equal_linspace();
    product();
    bad_product();
}

Running this with the assert_eq!(a, a_other) gives the following output

   Compiling rust-test v0.1.0 ($HOME/rust-test)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.32s
     Running `target/debug/rust-test`
1, 1
1, 2

thread 'main' panicked at src/main.rs:81:13:
assert_abs_diff_eq!(prod, a1.product(), epsilon = 1e-5)

    left  = 13454.566
    right = 13454.568


stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/17067e9ac6d7ecb70e50f92c1944e545188d2359/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/17067e9ac6d7ecb70e50f92c1944e545188d2359/library/core/src/panicking.rs:75:14
   2: rust_test::bad_product
             at ./src/main.rs:81:13
   3: rust_test::main
             at ./src/main.rs:89:5
   4: core::ops::function::FnOnce::call_once
             at $HOME/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

If anyone is feeling masochistic, please feel free to hunt this bug down - I'll come back to it as soon as I can, but there's some big life stuff going on very soon so I'll be a little busy.