Fix bug #126: handle NaNs correctly in min() and max()

Author: mgottscho

src/bfloat.rs

@@ -511,7 +511,7 @@ impl bf16 {
     #[inline]
     #[must_use]
     pub fn max(self, other: bf16) -> bf16 {
-        if other > self && !other.is_nan() {
+        if self.is_nan() || other > self {
             other
         } else {
             self
@@ -534,7 +534,7 @@ impl bf16 {
     #[inline]
     #[must_use]
     pub fn min(self, other: bf16) -> bf16 {
-        if other < self && !other.is_nan() {
+        if self.is_nan() || other < self {
             other
         } else {
             self
@@ -1877,4 +1877,50 @@ mod test {
             f.0 == roundtrip.0
         }
     }
+    
+    #[test]
+    fn test_max() {
+        let a = bf16::from_f32(0.0);
+        let b = bf16::from_f32(42.0);
+        assert_eq!(a.max(b), b);
+        
+        let a = bf16::from_f32(42.0);
+        let b = bf16::from_f32(0.0);
+        assert_eq!(a.max(b), a);
+
+        let a = bf16::NAN;
+        let b = bf16::from_f32(42.0);
+        assert_eq!(a.max(b), b);
+
+        let a = bf16::from_f32(42.0);
+        let b = bf16::NAN;
+        assert_eq!(a.max(b), a);
+
+        let a = bf16::NAN;
+        let b = bf16::NAN;
+        assert!(a.max(b).is_nan());
+    }
+
+    #[test]
+    fn test_min() {
+        let a = bf16::from_f32(0.0);
+        let b = bf16::from_f32(42.0);
+        assert_eq!(a.min(b), a);
+        
+        let a = bf16::from_f32(42.0);
+        let b = bf16::from_f32(0.0);
+        assert_eq!(a.min(b), b);
+
+        let a = bf16::NAN;
+        let b = bf16::from_f32(42.0);
+        assert_eq!(a.min(b), b);
+
+        let a = bf16::from_f32(42.0);
+        let b = bf16::NAN;
+        assert_eq!(a.min(b), a);
+
+        let a = bf16::NAN;
+        let b = bf16::NAN;
+        assert!(a.min(b).is_nan());
+    }
 }

src/binary16.rs

@@ -522,7 +522,7 @@ impl f16 {
     #[inline]
     #[must_use]
     pub fn max(self, other: f16) -> f16 {
-        if other > self && !other.is_nan() {
+        if self.is_nan() || other > self {
             other
         } else {
             self
@@ -545,7 +545,7 @@ impl f16 {
     #[inline]
     #[must_use]
     pub fn min(self, other: f16) -> f16 {
-        if other < self && !other.is_nan() {
+        if self.is_nan() || other < self {
             other
         } else {
             self
@@ -1961,4 +1961,50 @@ mod test {
             f.0 == roundtrip.0
         }
     }
+
+    #[test]
+    fn test_max() {
+        let a = f16::from_f32(0.0);
+        let b = f16::from_f32(42.0);
+        assert_eq!(a.max(b), b);
+        
+        let a = f16::from_f32(42.0);
+        let b = f16::from_f32(0.0);
+        assert_eq!(a.max(b), a);
+
+        let a = f16::NAN;
+        let b = f16::from_f32(42.0);
+        assert_eq!(a.max(b), b);
+
+        let a = f16::from_f32(42.0);
+        let b = f16::NAN;
+        assert_eq!(a.max(b), a);
+
+        let a = f16::NAN;
+        let b = f16::NAN;
+        assert!(a.max(b).is_nan());
+    }
+
+    #[test]
+    fn test_min() {
+        let a = f16::from_f32(0.0);
+        let b = f16::from_f32(42.0);
+        assert_eq!(a.min(b), a);
+        
+        let a = f16::from_f32(42.0);
+        let b = f16::from_f32(0.0);
+        assert_eq!(a.min(b), b);
+
+        let a = f16::NAN;
+        let b = f16::from_f32(42.0);
+        assert_eq!(a.min(b), b);
+
+        let a = f16::from_f32(42.0);
+        let b = f16::NAN;
+        assert_eq!(a.min(b), a);
+
+        let a = f16::NAN;
+        let b = f16::NAN;
+        assert!(a.min(b).is_nan());
+    }
 }