[Bug] Test failure for default feature on armel arch

[werdahias]

Hi,
I packaged your crate for debian. On armel two test fail because of what looks like an integer overflow for me:

...

test tests::system::signed::f64::abs ... ok
test tests::system::signed::f64::neg ... ok
test si::area::tests::f32::check_units ... FAILED
test si::volume::tests::f32::check_units ... FAILED

failures:

---- si::area::tests::f32::check_units stdout ----
thread 'si::area::tests::f32::check_units' panicked at 'assertion failed: `(left == right)`
  left: `0.0`,
 right: `1e-42`', src/tests/mod.rs:113:28
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:142:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:181:5
   4: uom::tests::test_trait::f32::<impl uom::tests::Test for f32>::assert_eq
             at ./src/tests/mod.rs:113:28
   5: <uom::si::Quantity<D,U,V> as uom::tests::Test>::assert_eq
             at ./src/system.rs:1298:17
   6: uom::si::area::tests::f32::check_units::test
             at ./src/si/area.rs:119:17
   7: uom::si::area::tests::f32::check_units
             at ./src/si/area.rs:111:13
   8: uom::si::area::tests::f32::check_units::{{closure}}
             at ./src/si/area.rs:87:9
   9: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  10: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

---- si::volume::tests::f32::check_units stdout ----
thread 'si::volume::tests::f32::check_units' panicked at 'assertion failed: `(left == right)`
  left: `0.0`,
 right: `1e-45`', src/tests/mod.rs:113:28
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.63.0/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:142:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /usr/src/rustc-1.63.0/library/core/src/panicking.rs:181:5
   4: uom::tests::test_trait::f32::<impl uom::tests::Test for f32>::assert_eq
             at ./src/tests/mod.rs:113:28
   5: <uom::si::Quantity<D,U,V> as uom::tests::Test>::assert_eq
             at ./src/system.rs:1298:17
   6: uom::si::volume::tests::f32::check_units::test
             at ./src/si/volume.rs:192:17
   7: uom::si::volume::tests::f32::check_units
             at ./src/si/volume.rs:182:13
   8: uom::si::volume::tests::f32::check_units::{{closure}}
             at ./src/si/volume.rs:160:9
   9: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
  10: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.63.0/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    si::area::tests::f32::check_units
    si::volume::tests::f32::check_units

test result: FAILED. 414 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.27s

error: test failed, to rerun pass '--lib'
...

I can patch those out for now, just though I'd let you know even if it's a very specific edge case.

[iliekturtles]

Thanks for the report. Looks like different floating point behavior on arm vs x86_64.

[werdahias]

tbh for now the easiest fix imho is to add a #[cfg(not(arch='arm'))] for those tests. I'll write a patch for it and get back to you.

[werdahias]

this patch should work imho:

diff --git a/src/si/area.rs b/src/si/area.rs
index b28ea30..89ab26d 100644
--- a/src/si/area.rs
+++ b/src/si/area.rs
@@ -84,6 +84,7 @@ mod tests {
         }
 
         #[test]
+        #[cfg(not(target_arch = "arm"))]
         fn check_units() {
             // Values too large for f32.
             if TypeId::of::<f64>() == TypeId::of::<V>() {
diff --git a/src/si/volume.rs b/src/si/volume.rs
index 6ee4d34..ff7e194 100644
--- a/src/si/volume.rs
+++ b/src/si/volume.rs
@@ -157,6 +157,7 @@ mod tests {
         }
 
         #[test]
+        #[cfg(not(target_arch = "arm"))]
         fn check_units() {
             // Values too large for f32.
             if TypeId::of::<f64>() == TypeId::of::<V>() {

[werdahias]

armel and armhf are seen as the same by the target_arch macro

[iliekturtles]

Note to the future: see if Github actions has an ARM host to include in the test matrix. Doesn't look right it right now.

[werdahias]

The debian CI runners check out; but I'd have liked to test the patch before uploading

[adamreichold]

Note to the future: see if Github actions has an ARM host to include in the test matrix. Doesn't look right it right now.

I think most people rely on QEMU for cross testing on GitHub actions, e.g. via its process-level emulation. This is also packaged up in cross-rs which is for example integration into actions-rs/cargo. All with the caveat that emulation might not show specific hardware quirks as real hardware does.

[iliekturtles]

I think most people rely on QEMU for cross testing on GitHub actions, e.g. via its process-level emulation.

Anyone happen to have QEMU setup to do a quick test and see if it shows the original issue @werdahias reported?

[adamreichold]

This needs docker on the $PATH (but alias docker=podman works as well), but it seems to work out of the box and seems to would have caught the issue:

> cargo install cross --git https://github.com/cross-rs/cross
[..]
> cross test --target arm-unknown-linux-gnueabi -- si::area::tests::f32::check_units
info: downloading component 'rust-std' for 'arm-unknown-linux-gnueabi'
info: installing component 'rust-std' for 'arm-unknown-linux-gnueabi'
 25.9 MiB /  25.9 MiB (100 %)  18.7 MiB/s in  1s ETA:  0s
Trying to pull ghcr.io/cross-rs/arm-unknown-linux-gnueabi:main...
Getting image source signatures
Copying blob 1698fcc01dce done  
Copying blob 6e9d123f1027 done  
Copying blob d5f6ccae57c7 done  
Copying blob 846c0b181fff done  
Copying blob 10e94c63f929 done  
Copying blob 1fba5f5b50bb done  
Copying blob 77862e5ac2c4 done  
Copying blob fdc4820d5264 done  
Copying blob 66f9b6373be6 done  
Copying blob 5064a6f99a6e done  
Copying blob a266babf7ba9 done  
Copying blob e8e973a198d0 done  
Copying blob dd0dac169f98 done  
Copying blob de81c7ff676f done  
Copying config b6515ed541 done  
Writing manifest to image destination
Storing signatures
   Compiling libc v0.2.139
   Compiling memchr v2.5.0
   Compiling cfg-if v1.0.0
   Compiling autocfg v1.1.0
   Compiling log v0.4.17
   Compiling typenum v1.16.0
   Compiling regex-syntax v0.6.28
   Compiling serde v1.0.152
   Compiling serde_json v1.0.91
   Compiling itoa v1.0.5
   Compiling ryu v1.0.12
   Compiling static_assertions v1.1.0
   Compiling num-traits v0.2.15
   Compiling aho-corasick v0.7.20
   Compiling getrandom v0.2.8
   Compiling rand_core v0.6.4
   Compiling rand v0.8.5
   Compiling regex v1.7.1
   Compiling approx v0.5.1
   Compiling env_logger v0.8.4
   Compiling quickcheck v1.0.3
   Compiling uom v0.33.0 (/home/adam/uom)
warning: unused import: `crate::lib::any::TypeId`
   --> src/si/volume.rs:109:13
    |
109 |         use crate::lib::any::TypeId;
    |             ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default

warning: `uom` (lib test) generated 1 warning (run `cargo fix --lib -p uom --tests` to apply 1 suggestion)
    Finished test [unoptimized + debuginfo] target(s) in 8.64s
     Running unittests src/lib.rs (/target/arm-unknown-linux-gnueabi/debug/deps/uom-26330e0968daef08)

running 1 test
test si::area::tests::f32::check_units ... FAILED

failures:

---- si::area::tests::f32::check_units stdout ----
thread 'si::area::tests::f32::check_units' panicked at 'assertion failed: `(left == right)`
  left: `0.0`,
 right: `1e-42`', src/tests/mod.rs:113:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    si::area::tests::f32::check_units

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 627 filtered out; finished in 0.04s

error: test failed, to rerun pass `--lib`

[iliekturtles]

@adamreichold Thanks!

I'm not super familiar with ARM. Does it do --ffast-math by default perhaps?

https://stackoverflow.com/questions/64036879/differing-floating-point-calculation-results-between-x86-64-and-armv8-2-a

[adamreichold]

Is 1e-42 a subnormal f32? I vaguely remember that ARM used to default to flush-to-zero because they need operating system support to handle subnormals (to keep the FPU simple).

[adamreichold]

It seems like it is, the f32 docs that the subnormals being at 1e-40.

[adamreichold]

I tried manipulating the FPSCR register, but that does not work on arm-unknown-linux-gnueabi as it lacks FPU registers by definition. But on arm-unknown-linux-gnueabihf the issue does not occur, i.e. this seems to be related to software floating point emulation.

[adamreichold]

I would not call it a fix, but it is a bit more targeted than disabling the offending tests:

diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index c6e705f..61c2008 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -108,10 +108,16 @@ mod test_trait {
         impl super::super::Test for V {
             /// Assert that `lhs` and `rhs` are exactly equal.
             fn assert_eq(lhs: &Self, rhs: &Self) {
-                match (lhs.is_nan(), rhs.is_nan()) {
-                    (true, true) => {}
-                    _ => { assert_eq!(lhs, rhs); }
+                use std::num::FpCategory::*;
+
+                match (lhs.classify(), rhs.classify()) {
+                    (Nan, Nan) => return,
+                    #[cfg(target_arch = "arm")]
+                    (Zero, Subnormal) | (Subnormal, Zero) => return,
+                    _ => (),
                 }
+
+                assert_eq!(lhs, rhs);
             }
 
             /// Assert that `lhs` and `rhs` are approximately equal for floating point types or