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

new(rs): fully by-hand expression simplification #273

Merged
merged 33 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f90f512
testing: filter out nAn-looking things in proptests
Jul 25, 2023
1fa8b22
fix: some more rules, big test commented out
Jul 26, 2023
3aae513
fix: use velcro, simplify and add some rules
Jul 27, 2023
72a2990
Merge branch 'main' into big-eggspression
Jul 27, 2023
ea6c596
fix: rename some rules
Jul 27, 2023
13bd8a9
fix: remove pathological test case; simplify ruleset
Jul 27, 2023
787c29b
Merge branch 'main' into big-eggspression
Aug 1, 2023
ec64583
fix: typo
Aug 3, 2023
cb76257
new(rs): combine by hand & egg to simplify expressions
Aug 4, 2023
1288663
feat: remove egg, rely solely on by_hand
Aug 31, 2023
645f7b4
fix: unused dep
Aug 31, 2023
890f591
Merge branch 'main' into combo-simplifier
Aug 31, 2023
994063a
fix: debug
Aug 31, 2023
513f361
fix: clippy
Aug 31, 2023
924c060
fix: un-remove hash utility
Aug 31, 2023
10e53dd
wip: still getting syntax errors on the affine case
Aug 31, 2023
956e1cf
fix: fewer references and clones
Aug 31, 2023
d666ae8
fix: get affine working
Aug 31, 2023
4d945cd
fix: π => 3.1415926535897932384626…
Sep 1, 2023
c311ecc
fix: various tidyings and comments
Sep 1, 2023
be60ad1
doc: justify LIMIT = 1
Sep 1, 2023
c2b60de
fix: respond to PR review comments
Sep 5, 2023
a6ae87b
fix: some other opportunities for simplification
Sep 7, 2023
3b1957f
fix: responding to PR review comments
Sep 7, 2023
60ffad9
fix: test coverage
Sep 7, 2023
72ea53b
fix: make private something that no longer needs pub(crate)
Sep 7, 2023
5ba3c3d
Merge branch 'main' into combo-simplifier
Sep 7, 2023
ab09e46
fix: responding to PR comments
Sep 9, 2023
44ba8ff
Merge branch 'main' into combo-simplifier
Sep 9, 2023
b3d735d
fix: Commentary tweaks per review comments
Sep 11, 2023
65ac565
fix: simplify exponentiations of numbers or π
Sep 11, 2023
3068fe4
fix: remove erroneous subtraction rule
Sep 11, 2023
100345a
fix: add simple double subtraction test to protect against previous b…
Sep 11, 2023
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
72 changes: 2 additions & 70 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]
pub(crate) 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
Loading