Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions quil-rs/proptest-regressions/expression/mod.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) })
30 changes: 29 additions & 1 deletion quil-rs/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand All @@ -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>,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down
Loading