JuliaAlgebra / DynamicPolynomials.jl

Multivariate polynomials implementation of commutative and non-commutative variables
Other
60 stars 20 forks source link

Is conversion from number to Polynomial supposed to work? #143

Closed shashi closed 10 months ago

shashi commented 10 months ago
julia> convert(DynamicPolynomials.Polynomial{true, Integer}, 2)
ERROR: MethodError: no method matching iscomm(::Bool)

Closest candidates are:
  iscomm(::Type{<:DynamicPolynomials.Commutative})
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/var.jl:103
  iscomm(::Type{<:DynamicPolynomials.NonCommutative})
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/var.jl:112
  iscomm(::Type{DynamicPolynomials.Variable{C}}) where C
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/var.jl:146
  ...

Stacktrace:
 [1] MonomialVector{true, Integer}(vars::Vector{DynamicPolynomials.Variable{true, Integer}}, Z::Vector{Vector{Int64}})
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/monomial_vector.jl:12
 [2] convert(#unused#::Type{Polynomial{true, Integer, Int64}}, t::MultivariatePolynomials.Term{Int64, Monomial{true, Integer}})
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/poly.jl:82
 [3] polynomial!(::MultivariatePolynomials.Term{Int64, Monomial{true, Integer}})
   @ MultivariatePolynomials ~/.julia/packages/MultivariatePolynomials/ckbfK/src/polynomial.jl:84
 [4] polynomial(::MultivariatePolynomials.Term{Int64, Monomial{true, Integer}})
   @ MultivariatePolynomials ~/.julia/packages/MultivariatePolynomials/ckbfK/src/polynomial.jl:53
 [5] convert(#unused#::Type{Polynomial{true, Integer}}, t::MultivariatePolynomials.Term{Int64, Monomial{true, Integer}})
   @ DynamicPolynomials ~/.julia/packages/DynamicPolynomials/W9SrI/src/promote.jl:53
 [6] convert_constant(#unused#::Type{Polynomial{true, Integer}}, α::Int64)
   @ MultivariatePolynomials ~/.julia/packages/MultivariatePolynomials/ckbfK/src/conversion.jl:7
 [7] convert(#unused#::Type{Polynomial{true, Integer}}, α::Int64)
   @ MultivariatePolynomials ~/.julia/packages/MultivariatePolyno

I think this used to work but a recent version has broken it, and broken Symbolics CI. I'm fixing it by making the output Vector{Any} instead of Vector{Polynomial}

shashi commented 10 months ago

Actually my fix does not work since the next sequence of code expects polynomials.

shashi commented 10 months ago

I see the type parameters are different now.

    polynoms = Vector{DP.Polynomial{true, commontype}}(undef, length(sympolys))

this is the offending line and there might be other broken things.

It seems the CompatHelper was not watching this
https://github.com/JuliaSymbolics/Symbolics.jl/pull/965

blegat commented 10 months ago

Yes, I changed the type, you should use polynomial_type(x, Float64) to get the type of polynomials or you can do something like https://github.com/jump-dev/PolyJuMP.jl/blob/master/src/nl_to_polynomial.jl#L4-L8

shashi commented 10 months ago

thanks!