Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiplication with different algebras #55

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Allow multiplication with different algebras #55

wants to merge 2 commits into from

Conversation

blegat
Copy link
Member

@blegat blegat commented Jul 3, 2024

@kalmarek
Copy link
Collaborator

kalmarek commented Jul 4, 2024

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
Copy link
Member Author

blegat commented Jul 5, 2024

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
Copy link
Collaborator

kalmarek commented Jul 5, 2024

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
Copy link
Member Author

blegat commented Jul 6, 2024

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
Copy link
Collaborator

kalmarek commented Jul 6, 2024

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...

Copy link

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            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 Coverage Δ
unittests 86.24% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blegat
Copy link
Member Author

blegat commented Aug 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants