JuliaAlgebra / DynamicPolynomials.jl

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

Variables get lost in Substitution #92

Closed manuelbb-upb closed 2 years ago

manuelbb-upb commented 2 years ago

Hi, I noticed something strange: When there are variables with a zero coefficient and one performs substitution, these variables get lost.

MWE:

using DynamicPolynomials
@polyvar x[1:2]
p = 1 * x[1] + 0 * x[2]
variables(p) == x

P = subs(p, x => x)
variables(P) == x[1]    # x[2] gets lost

My current dumb workaround is to add sum(0*x) after substitution to "restore" the variable information.

manuelbb-upb commented 2 years ago

I now have had a bit of time to look into it. This seems to be similar to https://github.com/JuliaAlgebra/DynamicPolynomials.jl/issues/80.

To fix it, I propose to change q = zero(Polynomial{C, Tout}) in this line to

q = zero_with_variables(Polynomial{C,Tout}, variables(p))

This makes the MWE work as expected.

@blegat Does that seem ok to you? If so, I can gladly make a PR :)

blegat commented 2 years ago

Yes, that's a good suggestion, PR welcome :)

votroto commented 2 years ago

Ouch, I guess it's in the spirit of "dynamicity", but I didn't expect this:

@polyvar x y
p = x + y
subs(p, x => 1) |> variables # returns [x, y]

I expected [y]. Is there a workaround?

blegat commented 2 years ago

You can use effective_variables

votroto commented 2 years ago

Good point, sorry for being cryptic. My current issue is that e.g. SumOfSquares.jl uses MultivariatePolynomials.jl and this commit breaks it slightly, as it uses all variables. Let's say I want to optimize a polynomial after making substitutions; How can I force the "expected" variables for the polynomial?

blegat commented 2 years ago

Good point, SumOfSquares should probably use effective_variables. Could you open an issue in SumOfSquares ? You would have the same issue with variables(x^2 + y^2 - y^2) being [x, y] so I think this change with substitution is consistent. I should have probably made a breaking release though, you're right.

manuelbb-upb commented 2 years ago

Ouch, I guess it's in the spirit of "dynamicity", but I didn't expect this

Just as a sidenote, I might on elaborate why I expected of variables to contain zero-coefficient variables and those substituted with constants:

Indeed, I have "dynamic" use case: polynomial interpolation / regression of data. Zero-coefficients might naturally occur. If the variables were not kept when there is a zero-coefficient then I would get an error when (lazily) evaluating the eventual interpolation polynomial because the length of the input feature vector and the number of variables mismatched.

The substitution issue showed up when I had a min-max-scaling polynomial for the features that accidentally also set some variables to zero when substituted into some of the interpolation polynomials and led to the same evaluation issues when plugging in the feature vectors.

Of course, the evaluation error can be avoided without the suggested fixes by always having the original variables around and doing a more verbose evaluation (in fact the way it is recommended, I believe):

using DynamicPolynomials
@polyvar x[1:2]
p1 = x[1]
subs( p1, x[1] => 1.0, x[2] => 1.0 )  # works as expected
p1( [1.0; 1.0] )  # does not work

I have mostly used the second notation because of its convenience (which is what I mean by "lazily" above) and because I guessed that the variable ordering is not an issue if I only use a single @polyvar call and a vector of variables. I am not familiar with the inner workings of the @polyvar macro but this comment seemed to confirm my thoughts.

blegat commented 2 years ago

Yes, the order is guaranteed;