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

Implement SubBasis #61

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

Implement SubBasis #61

wants to merge 4 commits into from

Conversation

kalmarek
Copy link
Collaborator

@kalmarek kalmarek commented Nov 7, 2024

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

Attention: Patch coverage is 63.33333% with 11 lines in your changes missing coverage. Please review.

Project coverage is 84.12%. Comparing base (bbcd649) to head (ad67331).

Files with missing lines Patch % Lines
src/bases_fixed.jl 63.33% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
- Coverage   85.93%   84.12%   -1.82%     
==========================================
  Files          14       13       -1     
  Lines         761      781      +20     
==========================================
+ Hits          654      657       +3     
- Misses        107      124      +17     
Flag Coverage Δ
unittests 84.12% <63.33%> (-1.82%) ⬇️

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.

k = findfirst(==(x), supp(b))
isnothing(k) && throw("x=$x is not supported on SubBasis")
@info T I
return convert(I, k)
Copy link
Member

@blegat blegat Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be convert(I, b.parent_basis[b.supporting_elts[x]]) because we store the indices

Base.IteratorSize(::Type{<:FixedBasis}) = Base.HasLength()
Base.length(b::FixedBasis) = length(b.elts)
struct SubBasis{T,I,V,B<:AbstractBasis{T,I}} <: FiniteSupportBasis{T,I}
supporting_elts::V
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the list of indices of the parent_basis. For DiracBasis, since indices are the same as elements, it works as well

@kalmarek
Copy link
Collaborator Author

@blegat I was pondering this topic over and over;
I have the feeling that the whole thing should be fixed on the level of coefficients instead of basis. And maybe that's why you should provide a few test applications, so that I can see where's the crux?

At the moment what we have is

  • B -- the parent basis
  • A1, A2 -- subbases of B (listing indices of B) and
  • algebra elements:
    • f1 contains (A1, v1) where v1 is the vector of coefficients wrt A1)
    • f2 contains (A2, v2).

We need to add/multiply f1 and f2, so we

  1. construct c1 a coefficients object from v1 and A1 (i.e. the one that implements nonzero_pairs)
  2. construct c2 for v2 and A2
  3. pass those into our mul!/add! functions
  • inside we need to construct sc::SparseCoefficients containing the result (this will be non-generic code btw. as preallocate in the generic is called only once and the result is stored directly there)
  1. construct A3 -- a subbasis of B from sc and the corresponding vector v3
  2. f3 needs to store now (A3, v3).

It seems that all of this can be achieved on the level of coefficients.
Why can't f1 already contain coefficients with respect to B which internally store (v1, list of corresponding indices of B)?

@blegat
Copy link
Member

blegat commented Nov 20, 2024

Good question. I definitely need an ExplicitBasis like SubBasis to represent a basis not necessarily used by an AlgebraElement but maybe we could consider that whenever you create an algebra element, you always use the parent ImplicitBasis. This is actually what I already do in MultivariateBases and SumOfSquares. I do all computation with implicit bases and then I just use explicit basis with coeffs.
The big blocker at the moment is the ability to represent elements from https://github.com/kalmarek/SymbolicWedderburn.jl/
We have a basis b1 of monomials of degree d and a basis b2 of monomials of degree up to 2d.
We do symmetry adapted basis for b1 -> a1 and a symmetry adapted basis for b2 -> a2.
We need to compute coeffs(a2, a1' * Q * a1).
So we should create a basis that concatenates a1 and a2 and have coeffs(a2, a1' * Q * a1) working.
If we can refactor https://github.com/kalmarek/SymbolicWedderburn.jl/blob/master/examples/sos_problem.jl using StarAlgebras then it means I can use that in SumOfSquares. At the moment, symmetry reduction in SumOfSquares has not been updated yet so that's the blocker to release a new version of SumOfSquares. Since in SumOfSquares, everything need to implement the StarAlgebras interface now, I need to implement coeffs(a2, a1' * Q * a1) using StarAlgebras so once we have a StarAlgebras basis for SymbolicWedderburn all lights will be green.
So the crux is https://github.com/kalmarek/SymbolicWedderburn.jl/blob/master/examples/sos_problem.jl / kalmarek/SymbolicWedderburn.jl#78 !

@kalmarek
Copy link
Collaborator Author

@blegat, I'm not sure what do you mean by

If we can refactor https://github.com/kalmarek/SymbolicWedderburn.jl/blob/master/examples/sos_problem.jl using StarAlgebras then it means I can use that in SumOfSquares.

Nowhere there do we form a product coeffs(a2, a1'*Q*a1) and that code after small tweaks should work with the newest StarAlgebras.

Here's some newly updated code that make use of DiracBasis and switches to FixedBasis only when particular (fixed) dimension relaxation is required:

import StarAlgebras as SA
let X = .... # AlgebraElement for a StarAlgebra of FPMonoid (infinite dimensional, DiracBasis)
    A = parent(elts)
    unit = one(A)

    # compute somehow a set of elements from `basis(A)` where dual sos-problem for X makes sense.
    linsupp, sizes = linear_supp(X, unit)
    B = SA.FixedBasis(linsupp, SA.mstructure(SA.basis(A)))
    
    # length(word(x)) corresponds to degree of polynomials
    N = maximum(x -> sizes[length(FPMonoids.word(x))÷2], linsupp)
        
    mt = let psd_basis = @view linsupp[1:N] # linsupp is ordered by word-length
        # here we use DiracMStructure of `B` and `mt` is `Matrix{<:Integer}`
        mt = [B[star(x)*y] for x in psd_basis, y in psd_basis]
    end

    model = JuMP.Model()
    # y is a vector of variables in basis `B`!
    y = @variable model y[1:length(B)]
    @objective model Max dot(coeffs(X, B), y) # max dot(X, y)
    @constraint model dot(coeffs(unit, B), y) == 1 # given a normalized `y`
    let P = @view y[mt]
        @constraint model P in PSDCone() # and mt defines a psd matrix
    end

    return model
end

see also https://github.com/kalmarek/QuantumStuff.jl/blob/mk/new_fpmonoids/src/psd_problems.jl

@blegat
Copy link
Member

blegat commented Nov 26, 2024

Thanks, I'll play around with while trying to update jump-dev/SumOfSquares.jl#375

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