Skip to content

Commit

Permalink
Apply NaN canonicalization in base tests (#586)
Browse files Browse the repository at this point in the history 
Fix for 
* dfinity/motoko#3647
* dfinity/motoko#4174

Enable NaN canonicalization for wasmtime test execution to get
consistent results across different processor platforms.
Adjust the unit tests and documenation regarding the propagation of the
NaN sign bit.

According to the Wasm specification and also the observed wasmtime
behavior (with NaN canonicalization), the NaN sign is only properly set
on `Float.abs`, `Float.neg`, and `Float.copySign`, not on the other
floating point operations.
  • Loading branch information
@luc-blaeser
luc-blaeser authored Aug 18, 2023
1 parent 4bb5717 commit 085468f
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 52 deletions.
19 changes: 15 additions & 4 deletions src/Float.mo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
/// * For absolute precision, it is recommened to encode the fraction number as a pair of a Nat for the base
/// and a Nat for the exponent (decimal point).
///
/// NaN sign:
/// * The NaN sign is only applied by `abs`, `neg`, and `copySign`. Other operations can have an arbitrary
/// sign bit for NaN results.

import Prim "mo:⛔";
import Int "Int";
Expand All @@ -59,7 +62,7 @@ module {
/// Notes:
/// * Equality test of `NaN` with itself or another number is always `false`.
/// * There exist many internal `NaN` value representations, such as positive and negative NaN,
/// signalling and quiet nans, each with many different bit representations.
/// signalling and quiet NaNs, each with many different bit representations.
///
/// Example:
/// ```motoko
Expand All @@ -77,7 +80,7 @@ module {
/// ```
/// abs(+inf) => +inf
/// abs(-inf) => +inf
/// abs(NaN) => NaN
/// abs(-NaN) => +NaN
/// abs(-0.0) => 0.0
/// ```
///
Expand Down Expand Up @@ -674,8 +677,16 @@ module {
/// Returns the negation of `x`, `-x` .
///
/// Changes the sign bit for infinity.
/// Issue: Inconsistent behavior for zero and `NaN`. Probably related to
/// https://github.com/dfinity/motoko/issues/3646
///
/// Special cases:
/// ```
/// neg(+inf) => -inf
/// neg(-inf) => +inf
/// neg(+NaN) => -NaN
/// neg(-NaN) => +NaN
/// neg(+0.0) => -0.0
/// neg(-0.0) => +0.0
/// ```
///
/// Example:
/// ```motoko
Expand Down
96 changes: 49 additions & 47 deletions test/Float.test.mo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ class Int64Testable(number : Int64) : T.TestableItem<Int64> {
let positiveInfinity = 1.0 / 0.0;
let negativeInfinity = -1.0 / 0.0;

let negativeNaN = 0.0 / 0.0;
let positiveNaN = Float.copySign(negativeNaN, 1.0); // Compiler issue, NaN are represented negative by default. https://github.com/dfinity/motoko/issues/3647
// Wasm specification: NaN signs are non-deterministic unless resulting from `copySign`, `abs`, or `neg`.
// With the NaN canonicalization mode, we get deterministic results for the NaN sign bit although it may
// be different to the expected result for floating point operations other than the ones mentioned before,
// e.g. `-0.0/0.0` results in a positive NaN with that canonicalization mode in wasmtime.
let positiveNaN = Float.copySign(0.0 / 0.0, 1.0);
let negativeNaN = Float.copySign(0.0 / 0.0, -1.0);

func isPositiveNaN(number : Float) : Bool {
debug_show (number) == "nan"
Expand Down Expand Up @@ -232,12 +236,12 @@ run(
test(
"positive NaN",
Float.abs(positiveNaN),
NaNMatcher()
PositiveNaNMatcher()
),
test(
"negative NaN",
Float.abs(negativeNaN),
NaNMatcher()
PositiveNaNMatcher()
)
]
)
Expand Down Expand Up @@ -2914,122 +2918,122 @@ run(
test(
"two positive NaNs",
Float.compare(positiveNaN, positiveNaN),
M.equals(OrderTestable(#equal)),
M.equals(OrderTestable(#equal))
),
test(
"two negative NaNs",
Float.compare(negativeNaN, negativeNaN),
M.equals(OrderTestable(#equal)),
M.equals(OrderTestable(#equal))
),
test(
"positive NaN, negative NaN",
Float.compare(positiveNaN, negativeNaN),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"negative NaN, positive NaN",
Float.compare(negativeNaN, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"number and positive NaN",
Float.compare(1.23, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"number and negative NaN",
Float.compare(1.23, negativeNaN),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"positive NaN and positive number",
Float.compare(positiveNaN, -1.23),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"positive NaN and negative number",
Float.compare(positiveNaN, -1.23),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"negative NaN and positive number",
Float.compare(negativeNaN, -1.23),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"negative NaN and negative number",
Float.compare(negativeNaN, -1.23),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"positive NaN and positive NaN",
Float.compare(positiveNaN, positiveNaN),
M.equals(OrderTestable(#equal)),
M.equals(OrderTestable(#equal))
),
test(
"negative NaN and positive NaN",
Float.compare(negativeNaN, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"positive NaN and negative NaN",
Float.compare(positiveNaN, negativeNaN),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"negative NaN and negative NaN",
Float.compare(negativeNaN, negativeNaN),
M.equals(OrderTestable(#equal)),
M.equals(OrderTestable(#equal))
),
test(
"positive NaN and positive infinity",
Float.compare(positiveNaN, positiveInfinity),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"positive NaN and negative infinity",
Float.compare(positiveNaN, negativeInfinity),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"positive NaN and positive infinity",
Float.compare(positiveNaN, positiveInfinity),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"negative NaN and negative infinity",
Float.compare(negativeNaN, negativeInfinity),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"negative NaN and positive infinity",
Float.compare(negativeNaN, positiveInfinity),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"positive infinity and positive NaN",
Float.compare(positiveInfinity, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"positive infinity and negative NaN",
Float.compare(positiveInfinity, negativeNaN),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
),
test(
"positive infinity and positive NaN",
Float.compare(positiveInfinity, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"negative infinity and positive NaN",
Float.compare(negativeInfinity, positiveNaN),
M.equals(OrderTestable(#less)),
M.equals(OrderTestable(#less))
),
test(
"negative infinity and negative NaN",
Float.compare(negativeInfinity, negativeNaN),
M.equals(OrderTestable(#greater)),
M.equals(OrderTestable(#greater))
)
]
)
Expand All @@ -3056,12 +3060,11 @@ run(
Float.neg(0.0),
M.equals(FloatTestable(0.0, noEpsilon))
),
// fails due to issue, probably related to https://github.com/dfinity/motoko/issues/3646
// test(
// "positive zero",
// Float.neg(positiveZero),
// NegativeZeroMatcher(),
// ),
test(
"positive zero",
Float.neg(positiveZero),
NegativeZeroMatcher()
),
test(
"negative zero",
Float.neg(negativeZero),
Expand All @@ -3087,17 +3090,16 @@ run(
Float.neg(negativeNaN),
NaNMatcher()
),
// Not working correctly, probably related to https://github.com/dfinity/motoko/issues/3646
// test(
// "positive NaN",
// Float.neg(positiveNaN),
// NegativeNaNMatcher(),
// ),
// test(
// "negative NaN",
// Float.neg(negativeNaN),
// PositiveNaNMatcher(),
// ),
test(
"positive NaN",
Float.neg(positiveNaN),
NegativeNaNMatcher()
),
test(
"negative NaN",
Float.neg(negativeNaN),
PositiveNaNMatcher()
)
]
)
);
Expand Down Expand Up @@ -3761,12 +3763,12 @@ run(
test(
"positive and negative zero",
Float.rem(positiveZero, negativeZero),
NegativeNaNMatcher()
NaNMatcher()
),
test(
"negative and positive zero",
Float.rem(negativeZero, positiveZero),
NegativeNaNMatcher()
NaNMatcher()
),
test(
"positive number and positive infinity",
Expand Down
2 changes: 1 addition & 1 deletion test/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

STDLIB ?= ../src
MOC ?= moc
WASMTIME_OPTIONS = --disable-cache
WASMTIME_OPTIONS = --disable-cache --enable-cranelift-nan-canonicalization

OUTDIR=_out

Expand Down

0 comments on commit 085468f

Please sign in to comment.