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

WIP: Various tweaks to the Rewriters #548

Merged
merged 24 commits into from
May 6, 2024
Merged

Conversation

shashi
Copy link
Member

@shashi shashi commented Aug 31, 2023

  • adds a ~MATCH slot which holds the whole expression
  • Adds a way for Chain to stop after finding a matching rule-useful when you want to chain rules by priority
  • fix metadata propagation in Walk
  • inspect & pluck (inspect & pluck #538) is included here so this PR depends on that one.

@shashi shashi changed the title Various tweaks to the Rewriters WIP: Various tweaks to the Rewriters Sep 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2023

Benchmark Results

master 1457894... master/1457894fe467c2...
overhead/acrule/a+2 0.742 ± 0.025 μs 0.723 ± 0.016 μs 1.03
overhead/acrule/a+2+b 0.717 ± 0.021 μs 0.702 ± 0.017 μs 1.02
overhead/acrule/a+b 0.259 ± 0.007 μs 0.253 ± 0.0075 μs 1.02
overhead/acrule/noop:Int 26.2 ± 0.91 ns 25.9 ± 0.05 ns 1.01
overhead/acrule/noop:Sym 0.0342 ± 0.0059 μs 0.0371 ± 0.0058 μs 0.924
overhead/rule/noop:Int 0.0373 ± 0.00034 μs 0.0438 ± 0.0011 μs 0.851
overhead/rule/noop:Sym 0.0455 ± 0.0018 μs 0.055 ± 0.0021 μs 0.828
overhead/rule/noop:Term 0.045 ± 0.0013 μs 0.0557 ± 0.0027 μs 0.808
overhead/ruleset/noop:Int 0.127 ± 0.0025 μs 0.127 ± 0.0035 μs 1
overhead/ruleset/noop:Sym 0.144 ± 0.003 μs 0.161 ± 0.0046 μs 0.894
overhead/ruleset/noop:Term 6.81 ± 0.41 μs 7.21 ± 0.43 μs 0.946
overhead/simplify/noop:Int 0.149 ± 0.0012 μs 0.15 ± 0.0013 μs 0.994
overhead/simplify/noop:Sym 0.151 ± 0.0032 μs 0.17 ± 0.0057 μs 0.887
overhead/simplify/noop:Term 0.0436 ± 0.0025 ms 0.0454 ± 0.0028 ms 0.96
overhead/simplify/randterm (+, *):serial 0.134 ± 0.0068 s 0.136 ± 0.0059 s 0.981
overhead/simplify/randterm (+, *):thread 0.0838 ± 0.028 s 0.0808 ± 0.028 s 1.04
overhead/simplify/randterm (/, *):serial 0.264 ± 0.0093 ms 0.274 ± 0.01 ms 0.964
overhead/simplify/randterm (/, *):thread 0.318 ± 0.011 ms 0.309 ± 0.011 ms 1.03
overhead/substitute/a 0.124 ± 0.0032 ms 0.123 ± 0.0033 ms 1.01
overhead/substitute/a,b 0.105 ± 0.0029 ms 0.104 ± 0.0031 ms 1.01
overhead/substitute/a,b,c 17.8 ± 0.76 μs 17.4 ± 0.78 μs 1.02
polyform/easy_iszero 0.0529 ± 0.0023 ms 0.0574 ± 0.0026 ms 0.922
polyform/isone 3.1 ± 0.01 ns 3.1 ± 0.009 ns 1
polyform/iszero 5.5 ± 0.078 ms 2.44 ± 0.049 ms 2.26
polyform/simplify_fractions 3.36 ± 0.051 ms 2.7 ± 0.049 ms 1.24
time_to_load 4.48 ± 0.015 s 4.49 ± 0.013 s 0.997

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

src/rewriters.jl Outdated
@@ -195,8 +201,9 @@ function (p::Walk{ord, C, F, false})(x) where {ord, C, F}
if ord === :pre
x = p.rw(x)
end
if iscall(x)
x = p.similarterm(x, operation(x), map(PassThrough(p), unsorted_arguments(x)))
if istree(x)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks incorrect right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@Vaibhavdixit02 Vaibhavdixit02 merged commit e39b109 into master May 6, 2024
6 of 11 checks passed
@Vaibhavdixit02 Vaibhavdixit02 deleted the s/rewriter-tweaks branch May 6, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants