implement coeff

[t-bltg]

Hi,

This is a proposition in order to obtain the coefficients from Symbolic expressions (focusing on monomials).

using Symbolics
@variables a x y

julia> coeff(2a, x)
0
julia> coeff(a*x, x)
a
julia> coeff(3x + 2y, x)
3
julia> coeff(x * y, x)
y
julia> coeff(x^2 + y, x^2)
1

Fix #216.
Fix #374.
Fix #236.

I've also rewritten degree as a nfc in the same pattern as coeff for consistency (enhance after #241).

Open question (which needs to be settled for coeff & degree): how do we handle SymbolicUtils.Div ?
degree(x / y, x) currently throws a StackOverflowError (added as a @test_broken in test/degree.jl).

EDIT: handling Div is outside the scope of this PR.

[bowenszhu]

I am wondering the expected results of the following cases.

coeff(((x + 1)^4 + x)^3, x^2)

coeff((x^2 - 1) / (x - 1), x)

coeff((x^(1//2) + y^0.5)^2, x)

[bowenszhu]

Open question (which needs to be settled for coeff & degree): how do we handle SymbolicUtils.Div ?

degree(x / y, x) currently throws a StackOverflowError (added as a @test_broken in test/degree.jl).

This is tricky. What do you expect for this case?

p = (y^2 + y + 1) / (y + 1)

[t-bltg]

Thanks for the comments.

coeff(((x + 1)^4 + x)^3, x^2)
coeff((x^2 - 1) / (x - 1), x)

These examples work if you expand or simplify the given expression prior to feeding coeff:

julia> ((x + 1)^4 + x)^3 |> expand |> e -> coeff(e, x^2)
93
julia> (x^2 - 1) / (x - 1) |> simplify |> e -> coeff(e, x)
1

I believe it should be up to the user to expand or simplify the given expression first before feeding coeff. Doing that in the background in coeff is not a good idea: e.g. when querying x, x^2, or more coeffs it would result in un-necessary duplicate (potentially costly) expand - simplify calls.

coeff((x^(1//2) + y^0.5)^2, x)

Thanks, I'll have a look: sympy returns 0 on that one too.

julia> (x^(1//2) + y^0.5)^2 |> expand |> e -> coeff(e, x)
1

Works after expansion too.

This is tricky. What do you expect for this case?

Agreed, so maybe let's focus on examples not involving quotients (monomials). This PR isn't meant to be exhaustive, but incremental.

Honestly, I don't this PR enhanced #241.

Maybe the wording wasn't right, but to me clarity of the code is an enhancement. Consistency is also imo a necessity. I'll rephrase that.
I also show in #685 that this PR enhances #241 from a performance pov.

[bowenszhu]

Honestly, I don't this PR enhanced #241.

Maybe the wording wasn't right, but to me clarity of the code is an enhancement. Consistency is also imo a necessity. I'll rephrase that. I also show in #685 that this PR enhances #241 from a performance pov.

True. Sorry about that.

[shashi]

Nice! Could you add doc strings for coeff and degree. I think we don't want to export degree anymore since it conflicts with exported names in another commonly used package.

[t-bltg]

I think we don't want to export degree anymore since it conflicts with exported names in another commonly used package.

We have the same problem of too many exported symbols in Plots.jl ;) Removed.

Could you add doc strings for coeff and degree.

Added.

[codecov-commenter]

Codecov Report

Merging #677 (ebdb7e4) into master (5b61308) will decrease coverage by 0.05%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master     #677      +/-   ##
==========================================
- Coverage   76.99%   76.93%   -0.06%     
==========================================
  Files          23       23              
  Lines        2712     2710       -2     
==========================================
- Hits         2088     2085       -3     
- Misses        624      625       +1     

Impacted Files	Coverage Δ
src/Symbolics.jl	71.42% <ø> (ø)
src/utils.jl	75.78% <95.23%> (-1.15%)	⬇️
src/semipoly.jl	96.66% <0.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[bowenszhu]

resolve #612

[shashi]

Thank you!!