JuliaAlgebra / StarAlgebras.jl

A package for computation in *-algebras with basis
MIT License
7 stars 3 forks source link

Allow multiplication with different algebras #55

Open blegat opened 4 months ago

blegat commented 4 months ago

Useful for https://github.com/kalmarek/SymbolicWedderburn.jl/issues/79

kalmarek commented 4 months ago

As expressed in our long call I think we need something better here, namely we need something (a struct)? that is going to combine Gramm (psd) constraint basis, Gramm matrix and (linear) constraint basis. What is the actual code that needs these? I bet (not too much though :D) it is not a simple * aka b'*Q*b, since the whole operation is treating Q as a quadratic form with the appropriate basis. Can you think of defining QuadraticForm struct that will do your bidding?

blegat commented 4 months ago

I don't know if we need to add the quadratic form in StarAlgebras. In SumOfSquares, I simply redirect operate!(::UnsafeAddMul, a::AlgebraElement, a::GramMatrix) to operate!(::UnsafeAddMul, a::AlgebraElement, b::AlgebraElement, c::AlgebraElement): https://github.com/jump-dev/SumOfSquares.jl/blob/cf146a6129adc388b64f4c77aac0c172353b6664/src/gram_matrix.jl#L147-L165 So a way to deal with algebra elements of different bases is all we need. At the moment, I use SparseCoefficient and FullBasis for a, b and c so that they have the same basis. If we have a multiplicative structure working with different basis, I could have a be in the constraint basis with a Vector or SparseVector coefficient and b/c be in the gram basis with SparseVector coefficient of one entry.

kalmarek commented 4 months ago

how about redirecting it to operate!(SA.UnsafeQuadratic(*), res::AlgebraElement, b::AlgebraElement, c::AlgebraElement) ? btw. what are args here https://github.com/jump-dev/SumOfSquares.jl/blob/cf146a6129adc388b64f4c77aac0c172353b6664/src/gram_matrix.jl#L151 ?

UnsafeQuadratic is explicit in what it does and unless we have a good reason, I'd avoid further overloading operate!(::UnsafeAddMul, ...) as it makes the code very fragile and hard to reason about. That is unless we change the algebraic model of our data.

So far we have assumed that Basis contains everything what is needed for algebra structure (both + and *). If this assumption does not hold for this particular case, I'd suggest that we need a new structure (QuadraticForm?) that encapsulates this use. It should contain both the input and output bases and propagate this information down the chain.

blegat commented 4 months ago

The args can be AlgebraElements. For instance, in Putinar's certificate, you have p = s0 + sum si fi so fi is in args.... For this reason, quadratic would not be enough but actually we also don't do symmetry reduction for Putinar's certificate yet anyway. But I was thinking of something that would also work without symmetry reduction so that's still relevant. So we would have

operate_to(::UnsafeAddMul, res::AlgebraElement, s::GramMatrix, f::AlgebraElement, b::AlgebraElement)

that becomes

operate_to(::UnsafeAddMul, res::AlgebraElement, a::AlgebraElement, b::AlgebraElement, f::AlgebraElement)

where each AlgebraElement would have basis SubBasis.

kalmarek commented 4 months ago

As I said, I'm against overloading the poor UnsafeAddMul with more meanings. If You're doing Putinar thing, we should be able to pass UnsafePutinarThing(*, inbasis, outbasis). If we're doing Gramm thing, lets pass UnsafeQuadraticThing(*, inbasis, outbasis). We could create some AbstractAddMul and supertype the three above if there's too much code overlap. But I honestly don't think there will be that much code overlap when it comes to performant code...

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.24%. Comparing base (ba3697f) to head (8a6e2dd).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #55 +/- ## ========================================== - Coverage 86.26% 86.24% -0.02% ========================================== Files 14 14 Lines 757 756 -1 ========================================== - Hits 653 652 -1 Misses 104 104 ``` | [Flag](https://app.codecov.io/gh/JuliaAlgebra/StarAlgebras.jl/pull/55/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaAlgebra) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/JuliaAlgebra/StarAlgebras.jl/pull/55/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaAlgebra) | `86.24% <100.00%> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaAlgebra#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

blegat commented 2 months ago

This PR, in addition to allowing to deal with SymbolicWedderburn decomposition by defining a custom MultiplicativeStructure, also allow doing https://github.com/JuliaAlgebra/MultivariateBases.jl/pull/50, which suggest it might be a good direction :)