fix!: make NaN == NaN inside Expression (#318)

Merge pull request #318 from rigetti/316-expression-proptest-failure
rigetti · Jan 14, 2025 · dde679f · dde679f
2 parents a169b43 + 2eba287
commit dde679f
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/quil-rs/proptest-regressions/expression/mod.txt b/quil-rs/proptest-regressions/expression/mod.txt
@@ -8,3 +8,4 @@ cc 4c32128d724ed0f840715fae4e194c99262dc153c64be39d2acf45b8903b20f7 # shrinks to
 cc 5cc95f2159ad7120bbaf296d3a9fb26fef30f57b61e76b3e0dc99f4759009fdb # shrinks to e = Number(Complex { re: 0.0, im: -2.772221265116396 })
 cc de70a1853ccef983fac85a87761ba08bfb2d54b2d4e880d5d90e7b4a75ecafb5 # shrinks to e = Address(MemoryReference { name: "mut", index: 0 })
 cc 9ad50859b68cb403ce1a67af0feef1f55d25587466878e364ba2810be5910b14 # shrinks to e = Address(MemoryReference { name: "iNf", index: 0 })
+cc 2b6281fa61604062f364d8e92d4b4e2753352d5ae1e76b7753bcb1201e8912b4 # shrinks to e = Infix(InfixExpression { left: Variable("ra"), operator: Slash, right: FunctionCall(FunctionCallExpression { function: Sine, expression: PiConstant }) })
diff --git a/quil-rs/src/expression/mod.rs b/quil-rs/src/expression/mod.rs
@@ -51,6 +51,10 @@ pub enum EvaluationError {
     NotANumber,
 }
 
+/// The type of Quil expressions.
+///
+/// Note that when comparing Quil expressions, any embedded NaNs are treated as *equal* to other
+/// NaNs, not unequal, in contravention of the IEEE 754 spec.
 #[derive(Clone, Debug)]
 pub enum Expression {
     Address(MemoryReference),
@@ -62,6 +66,10 @@ pub enum Expression {
     Variable(String),
 }
 
+/// The type of function call Quil expressions, e.g. `sin(e)`.
+///
+/// Note that when comparing Quil expressions, any embedded NaNs are treated as *equal* to other
+/// NaNs, not unequal, in contravention of the IEEE 754 spec.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct FunctionCallExpression {
     pub function: ExpressionFunction,
@@ -77,6 +85,10 @@ impl FunctionCallExpression {
     }
 }
 
+/// The type of infix Quil expressions, e.g. `e1 + e2`.
+///
+/// Note that when comparing Quil expressions, any embedded NaNs are treated as *equal* to other
+/// NaNs, not unequal, in contravention of the IEEE 754 spec.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct InfixExpression {
     pub left: Box<Expression>,
@@ -94,6 +106,10 @@ impl InfixExpression {
     }
 }
 
+/// The type of prefix Quil expressions, e.g. `-e`.
+///
+/// Note that when comparing Quil expressions, any embedded NaNs are treated as *equal* to other
+/// NaNs, not unequal, in contravention of the IEEE 754 spec.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct PrefixExpression {
     pub operator: PrefixOperator,
@@ -115,7 +131,10 @@ impl PartialEq for Expression {
         match (self, other) {
             (Self::Address(left), Self::Address(right)) => left == right,
             (Self::Infix(left), Self::Infix(right)) => left == right,
-            (Self::Number(left), Self::Number(right)) => left == right,
+            (Self::Number(left), Self::Number(right)) => {
+                (left.re == right.re || left.re.is_nan() && right.re.is_nan())
+                    && (left.im == right.im || left.im.is_nan() && right.im.is_nan())
+            }
             (Self::Prefix(left), Self::Prefix(right)) => left == right,
             (Self::FunctionCall(left), Self::FunctionCall(right)) => left == right,
             (Self::Variable(left), Self::Variable(right)) => left == right,
@@ -1010,6 +1029,7 @@ mod tests {
             prop_assert!(p.is_ok());
             let p = p.unwrap();
             let simple_p = p.clone().into_simplified();
+
             prop_assert_eq!(
                 simple_p.clone(),
                 simple_e.clone(),
@@ -1039,12 +1059,20 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_nan_is_equal() {
+        let left = Expression::Number(f64::NAN.into());
+        let right = left.clone();
+        assert_eq!(left, right);
+    }
+
     #[test]
     fn specific_simplification_tests() {
         for (input, expected) in [
             ("pi", Expression::Number(PI.into())),
             ("pi/2", Expression::Number((PI / 2.0).into())),
             ("pi * pi", Expression::Number((PI.powi(2)).into())),
+            ("1.0/(1.0-1.0)", Expression::Number(f64::NAN.into())),
             (
                 "(a[0]*2*pi)/6.283185307179586",
                 Expression::Address(MemoryReference {