JuliaAlgebra / MultivariatePolynomials.jl

Multivariate polynomials interface
https://juliaalgebra.github.io/MultivariatePolynomials.jl/stable/
Other
135 stars 27 forks source link

Complex-valued variables #213

Closed projekter closed 11 months ago

projekter commented 2 years ago

This is a first draft for the ideas mentioned in #212

blegat commented 1 year ago

Sorry for not following up on this, are you still interested ?

projekter commented 1 year ago

I am still interested, and if you also are, I'll go back and have a look at what I changed in the meantime (since I'm actively using those ideas locally for a package of mine) and update this request.

blegat commented 1 year ago

Ok, thanks, I'll have a look at the update

projekter commented 1 year ago

Now I updated this to my current state. And I also uploaded my current implementation of all this in DynamicPolynomials. Note that I did not include an implementation of Base.abs2, as it is not clear whether this should be done as conj(z)*z or as real(z)^2+imag(z)^2, which will give entirely different representations. Well... you could already debate this for real(z). If z = z_r + im*z_i, is this then z_r or rather (z+conj(z))/2? For polynomials/monomials, this will make a huge difference in how many terms there are.

blegat commented 1 year ago

Looks good, could you rebase and add the comment in the code ? Then I think it's good to merge and we'll make it part of https://github.com/JuliaAlgebra/MultivariatePolynomials.jl/issues/250

projekter commented 1 year ago

Done - the complex functionality should also appear somewhere in the documentation. Which are the correct places? I assume iscomplex -> Polynomial isrealpart, isimagpart, isconj, ordinary_variable -> Variable degree_complex, halfdegree -> Term mindegree_complex, minhalfdegree, maxdegree_complex, maxhalfdegree, extdegree_complex, exthalfdegree -> Polynomial

And I guess tests would also be a nice thing - but until there is a concrete implementation merged, this does not seam feasible?

blegat commented 1 year ago

Yes, the documentation places look good. You can add tests and change .github/workflow/ci.yml so that it uses your branch for DynamicPolynomials as I did in https://github.com/JuliaAlgebra/MultivariatePolynomials.jl/pull/231/commits/ecdd33fb09e4c6491654b4d16d650aea64b84b9d for instance

projekter commented 1 year ago

Whew, when trying to implement the tests, I needed to go on the current master. I hope I found all the necessary replacements to the new conventions. Locally, all the tests work apart from two broken ones. One is due to iszero not being implemented on MutableArithmetics.Zero (since I don't know much/anything about MA, I can't comment on this). The other one is due to a difficulty on parsing incomplete substitutions - which hopefully is not the standard case. This could be mitigated (though it's more a DynamicPolynomials issue), but at some conversion cost.

projekter commented 1 year ago

@blegat , is there something that still prevents this from being merged? Everything should be adapted for the current version, the test are working by now with the DP implementation (though you can always argue that there could be more tests), the functions are documented...

blegat commented 1 year ago

Thanks, no let's try to push this through the finish line.