leoyvens commented on PR #13590: URL: https://github.com/apache/datafusion/pull/13590#issuecomment-2507780521
There was the following test failure on amd64 and win64: ``` External error: query result mismatch: [SQL] select acos(0), acos(0.5), acos(1); [Diff] (-expected|+actual) - 1.5707963267948966 1.0471975511965976 0 + 1.5707963267948966 1.0471975511965979 0 at test_files/scalar.slt:93 ``` This is surfacing non-determinism across target platforms. This is expected behaviour for Rust std. For `acos`, and many other float math functions, the [Rust std docs](https://doc.rust-lang.org/std/primitive.f64.html#method.acos) say: > The precision of this function is non-deterministic. So I went looking to see if there was a performant but portable float math library we could use. [libm](https://github.com/rust-lang/libm) seems to be it, it's what rustc uses when targeting WASM. To gain confidence that we'd not be risking any significant performance regression, I analysed some benchmark results taken from the CI of the [metallic-rs project](https://github.com/jdh8/metallic-rs/tree/main) (credit to @jdh8). It benchmarks only f32, not f64. From this data, I made a chart comparing std and libm results:  libm and std seem to have similar performance. If we value portability, I'd propose that we switch to `libm`, which is what I've implemented in the second commit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
