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

Extend and document canonical form of Symbolic{<:Number} #266

Merged
merged 4 commits into from
Apr 15, 2021

Conversation

Suavesito-Olimpiada
Copy link
Contributor

fix #256

@Suavesito-Olimpiada
Copy link
Contributor Author

Suavesito-Olimpiada commented Apr 6, 2021

As I saw when writing this docs, there are things that can be done to further improve the canonical form just using Mul, Add and Pow. Some of these are "fusing" expressions of type Pow and Mul.

I can contribute these this week, but I don't know if put them in this PR or make another with "just the code". If is needed in another PR i would extend the docs here to reflect the changes made in the other PR.

I also don't know if I should add here in the docs the optimizations for some functions (as in 262) or not, and all the "basic" operators [+, -, \, /, ^, *].

@shashi
Copy link
Member

shashi commented Apr 6, 2021

Some of these are "fussing" expressions of type Pow and Mul.

What do you mean by this?

What are the changes you're planning to make? Feel free to put them in this PR! :)

@Suavesito-Olimpiada
Copy link
Contributor Author

Suavesito-Olimpiada commented Apr 6, 2021

Some of these are "fusing" expressions of type Pow and Mul.

What do you mean by this?

In master now you have that

julia> x*inv(x)
1

For some x expressions, but

julia> (x^-1)*inv(x^-1)
(x^-1)*((x^-1)^-1)

And you can see why this is happening

julia> print_tree((x^-1)*inv(x^-1))
*
├─ 1
├─ (^)
│  ├─ x
│  └─ -1
└─ (^)
   ├─ ^
   │  ├─ x
   │  └─ -1
   └─ -1

Basically Mul is not merging (what I called "fusing") with Pow.

@Suavesito-Olimpiada
Copy link
Contributor Author

Suavesito-Olimpiada commented Apr 6, 2021

I found what it is happening, the Mul does merge with Pow, but Pow doesn't reduce with another Pow.

@Suavesito-Olimpiada
Copy link
Contributor Author

Now with this PR, aside the documentation

julia> (x^-1)*inv(x^-1)
1

Still needs to finish the documentation part.

@Suavesito-Olimpiada Suavesito-Olimpiada changed the title Documentation for canonical form of Symbolic{<:Number} Extend and document canonical form of Symbolic{<:Number} Apr 6, 2021
@YingboMa
Copy link
Member

YingboMa commented Apr 7, 2021

Could you add a test for (x^-1)*inv(x^-1) in here

@testset "canonical form" begin

@Suavesito-Olimpiada
Copy link
Contributor Author

Yeah, sure. I finished adding the documentation. And I added the test.

@Suavesito-Olimpiada
Copy link
Contributor Author

Locally all the test passed 😄, now to the next issue. 😛

@shashi shashi merged commit 0d2f6b7 into JuliaSymbolics:master Apr 15, 2021
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.

Document canonical form of Number terms
3 participants