Skip to content

Commit

Permalink
new(rs): fully by-hand expression simplification (#273)
Browse files Browse the repository at this point in the history
* testing: filter out nAn-looking things in proptests

* fix: some more rules, big test commented out

* fix: use velcro, simplify and add some rules

* fix: rename some rules

* fix: remove pathological test case; simplify ruleset

* fix: typo

* new(rs): combine by hand & egg to simplify expressions

* feat: remove egg, rely solely on by_hand

* fix: unused dep

* fix: debug

* fix: clippy

* fix: un-remove hash utility

* wip: still getting syntax errors on the affine case

* fix: fewer references and clones

* fix: get affine working

* fix: π => 3.1415926535897932384626…

* fix: various tidyings and comments

* doc: justify LIMIT = 1

* fix: respond to PR review comments

* fix: some other opportunities for simplification

* fix: responding to PR review comments

* fix: test coverage

* fix: make private something that no longer needs pub(crate)

* fix: responding to PR comments

* fix: Commentary tweaks per review comments

* fix: simplify exponentiations of numbers or π

* fix: remove erroneous subtraction rule

* fix: add simple double subtraction test to protect against previous bad rule
  • Loading branch information
genos authored Sep 13, 2023
1 parent 882ffb3 commit 5a80c38
Show file tree
Hide file tree
Showing 7 changed files with 1,231 additions and 700 deletions.
87 changes: 1 addition & 86 deletions Cargo.lock

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

3 changes: 1 addition & 2 deletions quil-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ categories = ["parser-implementations", "science", "compilers", "emulators"]
[dependencies]
approx = { version = "0.5.1", features = ["num-complex"] }
dot-writer = { version = "0.1.2", optional = true }
egg = { version = "0.9.4", features = ["deterministic"] }
indexmap = "1.6.1"
itertools = "0.11.0"
lexical = "6.1.1"
Expand Down Expand Up @@ -40,7 +39,7 @@ rstest = "0.18.1"
# These are described in the crate README.md
[features]
graphviz-dot = ["dot-writer"]
wasm-bindgen = ["egg/wasm-bindgen"]
wasm-bindgen = []

[[bench]]
name = "parser"
Expand Down
28 changes: 15 additions & 13 deletions quil-rs/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,7 @@ impl Expression {
Expression::PiConstant => {
*self = Expression::Number(Complex64::from(PI));
}
_ => {
if let Ok(simpler) = simplification::run(self) {
*self = simpler;
}
}
_ => *self = simplification::run(self),
}
}

Expand Down Expand Up @@ -666,11 +662,19 @@ impl fmt::Display for InfixOperator {
#[allow(clippy::arc_with_non_send_sync)]
mod tests {
use super::*;
use crate::hash::hash_to_u64;
use crate::reserved::ReservedToken;
use proptest::prelude::*;
use std::collections::hash_map::DefaultHasher;
use std::collections::HashSet;

/// Hash value helper: turn a hashable thing into a u64.
#[inline]
fn hash_to_u64<T: Hash>(t: &T) -> u64 {
let mut s = DefaultHasher::new();
t.hash(&mut s);
s.finish()
}

#[test]
fn simplify_and_evaluate() {
use Expression::*;
Expand Down Expand Up @@ -790,7 +794,7 @@ mod tests {
// Better behaved than the auto-derived version for names
fn arb_name() -> impl Strategy<Value = String> {
r"[a-z][a-zA-Z0-9]{1,10}".prop_filter("Exclude reserved tokens", |t| {
ReservedToken::from_str(t).is_err()
ReservedToken::from_str(t).is_err() && !t.to_lowercase().starts_with("nan")
})
}

Expand Down Expand Up @@ -835,12 +839,10 @@ mod tests {
right: Box::new(r)
})
),
(any::<PrefixOperator>(), expr).prop_map(|(operator, e)| Prefix(
PrefixExpression {
operator,
expression: Box::new(e)
}
))
(expr).prop_map(|e| Prefix(PrefixExpression {
operator: PrefixOperator::Minus,
expression: Box::new(e)
}))
]
},
)
Expand Down
Loading

0 comments on commit 5a80c38

Please sign in to comment.