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

add Exp rules #80

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Roger-luo
Copy link

@Roger-luo Roger-luo commented Sep 24, 2019

however I find this might be conflict with BASIC rules which evals the pi * im expression, is there a way to treat Irrationals differently?

Update: I changed the is_constant and printings for Irrational, it looks working now! should fix #81

@codecov-io
Copy link

codecov-io commented Sep 24, 2019

Codecov Report

Merging #80 into master will increase coverage by 4.3%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #80     +/-   ##
=========================================
+ Coverage   88.47%   92.78%   +4.3%     
=========================================
  Files           8        8             
  Lines         269      291     +22     
=========================================
+ Hits          238      270     +32     
+ Misses         31       21     -10
Impacted Files Coverage Δ
src/rules.jl 92.18% <100%> (+0.95%) ⬆️
src/utils.jl 75% <100%> (+8.33%) ⬆️
src/types.jl 90.9% <50%> (+7.18%) ⬆️
src/match.jl 97.01% <0%> (+0.34%) ⬆️
src/rule.jl 98.59% <0%> (+4.65%) ⬆️
src/properties.jl 71.42% <0%> (+9.89%) ⬆️
src/context.jl 88.88% <0%> (+12.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4711ded...c6f3c40. Read the comment docs.

@Roger-luo
Copy link
Author

it seems the rule of exp(2π * im) => 1 is not working when combined with :BASIC, since the definitions is

julia> dump(r)
PatternRule
  left: Term
    ex: Expr
      head: Symbol call
      args: Array{Any}((2,))
        1: exp (function of type typeof(exp))
        2: Expr
          head: Symbol call
          args: Array{Any}((3,))
            1: * (function of type typeof(*))
            2: Expr
              head: Symbol call
              args: Array{Any}((3,))
                1: * (function of type typeof(*))
                2: Int64 2
                3: Irrational{} π = 3.1415926535897...
            3: Complex{Bool}
              re: Bool false
              im: Bool true
  right: Term
    ex: Int64 1
  ps: Array{Function}((0,))

but after :BASIC, the * expression will be treated as associative operator which is a list of all args instead of the binary tree.

@Roger-luo
Copy link
Author

I fixed this by defining it as exp(*(2, π, im)) => 1 but I'm wondering if there is some kind of better fix for associative operators?

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.

handling Irrationals
2 participants