diff --git a/Cargo.lock b/Cargo.lock index f6fc4065..6ece5293 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1130,7 +1130,7 @@ dependencies = [ [[package]] name = "quil-rs" -version = "0.20.0-rc.0" +version = "0.20.1-rc.1" dependencies = [ "approx", "clap", diff --git a/quil-rs/src/expression/mod.rs b/quil-rs/src/expression/mod.rs index 1bb8a173..7d8f74dc 100644 --- a/quil-rs/src/expression/mod.rs +++ b/quil-rs/src/expression/mod.rs @@ -13,10 +13,12 @@ // limitations under the License. use crate::{ - hash::{hash_f64, hash_to_u64}, + hash::hash_f64, + imag, + instruction::MemoryReference, parser::{lex, parse_expression, ParseError}, program::{disallow_leftover, ParseProgramError}, - {imag, instruction::MemoryReference, real}, + real, }; use lexical::{format, to_string_with_options, WriteFloatOptions}; use nom_locate::LocatedSpan; @@ -53,13 +55,13 @@ pub enum Expression { Address(MemoryReference), FunctionCall(FunctionCallExpression), Infix(InfixExpression), - Number(num_complex::Complex64), + Number(Complex64), PiConstant, Prefix(PrefixExpression), Variable(String), } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct FunctionCallExpression { pub function: ExpressionFunction, pub expression: Box, @@ -74,7 +76,7 @@ impl FunctionCallExpression { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct InfixExpression { pub left: Box, pub operator: InfixOperator, @@ -91,7 +93,7 @@ impl InfixExpression { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct PrefixExpression { pub operator: PrefixOperator, pub expression: Box, @@ -106,19 +108,34 @@ impl PrefixExpression { } } +impl PartialEq for Expression { + // Implemented by hand since we can't derive with f64s hidden inside. + fn eq(&self, other: &Self) -> bool { + 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::Prefix(left), Self::Prefix(right)) => left == right, + (Self::FunctionCall(left), Self::FunctionCall(right)) => left == right, + (Self::Variable(left), Self::Variable(right)) => left == right, + (Self::PiConstant, Self::PiConstant) => true, + _ => false, + } + } +} + +// Implemented by hand since we can't derive with f64s hidden inside. +impl Eq for Expression {} + impl Hash for Expression { // Implemented by hand since we can't derive with f64s hidden inside. - // Also to understand when things should be the same, like with commutativity (`1 + 2 == 2 + 1`). - // See https://github.com/rigetti/quil-rust/issues/27 fn hash(&self, state: &mut H) { - use std::cmp::{max_by_key, min_by_key}; - use Expression::*; match self { - Address(m) => { + Self::Address(m) => { "Address".hash(state); m.hash(state); } - FunctionCall(FunctionCallExpression { + Self::FunctionCall(FunctionCallExpression { function, expression, }) => { @@ -126,34 +143,19 @@ impl Hash for Expression { function.hash(state); expression.hash(state); } - Infix(InfixExpression { + Self::Infix(InfixExpression { left, operator, right, }) => { "Infix".hash(state); operator.hash(state); - match operator { - InfixOperator::Plus | InfixOperator::Star => { - // commutative, so put left & right in decreasing order by hash value - let (a, b) = ( - min_by_key(&left, &right, hash_to_u64), - max_by_key(&left, &right, hash_to_u64), - ); - a.hash(state); - b.hash(state); - } - _ => { - left.hash(state); - right.hash(state); - } - } + left.hash(state); + right.hash(state); } - Number(n) => { + Self::Number(n) => { "Number".hash(state); // Skip zero values (akin to `format_complex`). - // Also, since f64 isn't hashable, use the u64 binary representation. - // The docs claim this is rather portable: https://doc.rust-lang.org/std/primitive.f64.html#method.to_bits if n.re.abs() > 0f64 { hash_f64(n.re, state) } @@ -161,15 +163,15 @@ impl Hash for Expression { hash_f64(n.im, state) } } - PiConstant => { + Self::PiConstant => { "PiConstant".hash(state); } - Prefix(p) => { + Self::Prefix(p) => { "Prefix".hash(state); p.operator.hash(state); p.expression.hash(state); } - Variable(v) => { + Self::Variable(v) => { "Variable".hash(state); v.hash(state); } @@ -177,15 +179,6 @@ impl Hash for Expression { } } -impl PartialEq for Expression { - // Partial equality by hash value - fn eq(&self, other: &Self) -> bool { - hash_to_u64(self) == hash_to_u64(other) - } -} - -impl Eq for Expression {} - macro_rules! impl_expr_op { ($name:ident, $name_assign:ident, $function:ident, $function_assign:ident, $operator:ident) => { impl $name for Expression { @@ -215,11 +208,7 @@ impl_expr_op!(Mul, MulAssign, mul, mul_assign, Star); impl_expr_op!(Div, DivAssign, div, div_assign, Slash); /// Compute the result of an infix expression where both operands are complex. -fn calculate_infix( - left: &num_complex::Complex64, - operator: &InfixOperator, - right: &num_complex::Complex64, -) -> num_complex::Complex64 { +fn calculate_infix(left: &Complex64, operator: &InfixOperator, right: &Complex64) -> Complex64 { use InfixOperator::*; match operator { Caret => left.powc(*right), @@ -231,10 +220,7 @@ fn calculate_infix( } /// Compute the result of a Quil-defined expression function where the operand is complex. -fn calculate_function( - function: &ExpressionFunction, - argument: &num_complex::Complex64, -) -> num_complex::Complex64 { +fn calculate_function(function: &ExpressionFunction, argument: &Complex64) -> Complex64 { use ExpressionFunction::*; match function { Sine => argument.sin(), @@ -323,9 +309,9 @@ impl Expression { /// ``` pub fn evaluate( &self, - variables: &HashMap, + variables: &HashMap, memory_references: &HashMap<&str, Vec>, - ) -> Result { + ) -> Result { use Expression::*; match self { @@ -660,18 +646,11 @@ impl fmt::Display for InfixOperator { #[cfg(test)] mod tests { - use std::collections::{hash_map::DefaultHasher, HashSet}; - - use num_complex::Complex64; - use proptest::prelude::*; - - use crate::{ - expression::{EvaluationError, Expression, ExpressionFunction}, - real, - reserved::ReservedToken, - }; - use super::*; + use crate::hash::hash_to_u64; + use crate::reserved::ReservedToken; + use proptest::prelude::*; + use std::collections::HashSet; #[test] fn simplify_and_evaluate() { @@ -862,21 +841,6 @@ mod tests { prop_assert_ne!(&first, &differing); } - #[test] - fn eq_commutative(a in any::(), b in any::()) { - let first = Expression::Infix(InfixExpression { - left: Box::new(Expression::Number(real!(a))), - operator: InfixOperator::Plus, - right: Box::new(Expression::Number(real!(b))), - } ); - let second = Expression::Infix(InfixExpression { - left: Box::new(Expression::Number(real!(b))), - operator: InfixOperator::Plus, - right: Box::new(Expression::Number(real!(a))), - }); - prop_assert_eq!(first, second); - } - #[test] fn hash(a in any::(), b in any::()) { let first = Expression::Infix (InfixExpression { @@ -892,36 +856,9 @@ mod tests { assert!(!set.contains(&differing)) } - #[test] - fn hash_commutative(a in any::(), b in any::()) { - let first = Expression::Infix(InfixExpression { - left: Box::new(Expression::Number(real!(a))), - operator: InfixOperator::Plus, - right: Box::new(Expression::Number(real!(b))), - } ); - let second = Expression::Infix(InfixExpression { - left: Box::new(Expression::Number(real!(b))), - operator: InfixOperator::Plus, - right: Box::new(Expression::Number(real!(a))), - } ); - let mut set = HashSet::new(); - set.insert(first); - assert!(set.contains(&second)); - } - #[test] fn eq_iff_hash_eq(x in arb_expr(), y in arb_expr()) { - let h_x = { - let mut s = DefaultHasher::new(); - x.hash(&mut s); - s.finish() - }; - let h_y = { - let mut s = DefaultHasher::new(); - y.hash(&mut s); - s.finish() - }; - prop_assert_eq!(x == y, h_x == h_y); + prop_assert_eq!(x == y, hash_to_u64(&x) == hash_to_u64(&y)); } #[test] diff --git a/quil-rs/src/expression/simplification.rs b/quil-rs/src/expression/simplification.rs index fa17a50c..f784d787 100644 --- a/quil-rs/src/expression/simplification.rs +++ b/quil-rs/src/expression/simplification.rs @@ -1,11 +1,11 @@ /// Complex machinery for simplifying [`Expression`]s. use crate::{ expression::{ - format_complex, hash_to_u64, imag, is_small, real, Expression, ExpressionFunction, - FunctionCallExpression, InfixExpression, InfixOperator, MemoryReference, PrefixExpression, - PrefixOperator, + format_complex, is_small, Expression, ExpressionFunction, FunctionCallExpression, + InfixExpression, InfixOperator, MemoryReference, PrefixExpression, PrefixOperator, }, - hash::hash_f64, + hash::{hash_f64, hash_to_u64}, + imag, real, }; use egg::{define_language, rewrite as rw, Id, Language, RecExpr}; use once_cell::sync::Lazy;