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

Add support for complex interface #23

Open
blegat opened this issue Mar 1, 2023 · 23 comments · May be fixed by #28
Open

Add support for complex interface #23

blegat opened this issue Mar 1, 2023 · 23 comments · May be fixed by #28

Comments

@blegat
Copy link
Member

blegat commented Mar 1, 2023

See jump-dev/JuMP.jl#3257 (comment) for a good MWE to use. I won't have time to take a look this week though

@araujoms
Copy link
Contributor

I would be interested in adding proper complex support through MOI. I would be very helpful if you could give me some pointers on what needs to be done.

@blegat
Copy link
Member Author

blegat commented May 23, 2023

Thanks for offering your help. There are two things to add which I think could be added independently in order to divide and conquer:

  1. Support for VectorAffineFunction{Complex{Float64}}-inZeros
  2. Support for VectorAffineFunction{Float64}-inHermitianPSDCone

For 1), you have to modify
https://github.com/jump-dev/SeDuMi.jl/blob/master/src/MOI_wrapper.jl#L29-L43
into something like

MOI.Utilities.@product_of_sets(
    ComplexCones,
    MOI.Zeros,
)
MOI.Utilities.@struct_of_constraints_by_function_types(
    ComplexOrReal,
    MOI.VectorAffineFunction{Complex{Float64}},
    MOI.VectorAffineFunction{Float64},
)
const OptimizerCache = MOI.Utilities.GenericModel{
    Float64,
    MOI.Utilities.ObjectiveContainer{Float64},
    MOI.Utilities.VariablesContainer{Float64},
    ComplexOrReal{Float64}{
        MOI.Utilities.MatrixOfConstraints{
            Float64,
            MOI.Utilities.MutableSparseMatrixCSC{
                Float64,
                Int,
                MOI.Utilities.OneBasedIndexing,
            },
            Vector{Float64},
            ComplexCones{Float64},
        },
        MOI.Utilities.MatrixOfConstraints{
            Float64,
            MOI.Utilities.MutableSparseMatrixCSC{
                Float64,
                Int,
                MOI.Utilities.OneBasedIndexing,
            },
            Vector{Float64},
            Cones{Float64},
        },
    },
}

You should add

function MOI.supports_constraint(::Optimizer, ::Type{MOI.VectorAffineFunction{Complex{Float64}}}, ::Type{MOI.Zeros}})
    return true
end

Then, replace

Ac = src.constraints

by

Ac = MOI.Utilities.constraints(
    src.constraints,
    MOI.VectorAffineFunction{Float64},
    MOI.Zeros,
)

and then you can get the complex one with

Ac_complex = MOI.Utilities.constraints(
    src.constraints,
    MOI.VectorAffineFunction{Complex{Float64}},
    MOI.Zeros,
)

Then you can combine the two matrices and update xcomplex and ycomplex accordingly in the Cone struct.

@blegat
Copy link
Member Author

blegat commented May 23, 2023

Once 1) is done, for 2), I would define a bridge from VectorAffineFunction{Float64}-in-MOI.HermtianPSDCone to VectorAffineFunction{Complex{Float64}}-in-SeDuMi.ScaledPSDCone similar to https://github.com/jump-dev/SeDuMi.jl/blob/master/src/scaled_psd_cone_bridge.jl.
Then modify

MOI.Utilities.@product_of_sets(
    ComplexCones,
    MOI.Zeros,
)

into

MOI.Utilities.@product_of_sets(
    ComplexCones,
    MOI.Zeros,
    ScaledPSDCone
)

and modify

function MOI.supports_constraint(::Optimizer, ::Type{MOI.VectorAffineFunction{Complex{Float64}}}, ::Type{MOI.Zeros}})
    return true
end

into

function MOI.supports_constraint(::Optimizer, ::Type{MOI.VectorAffineFunction{Complex{Float64}}}, ::Type{<:Union{MOI.Zeros,ScaledPSDCone}}})
    return true
end

and then modify MOI.optimize! accordingly.

I would start with a PR for 1) that we merge before attacking 2). Let us know if you encounter any issue

@araujoms
Copy link
Contributor

Thanks a lot for the detailed pointers. I'm afraid I didn't understand everything. Isn't 1) just allowing complex numbers in the equality constraints? But what is the benefit of doing that directly in SeDuMi? What SeDuMi will do is just split the constraint into real and imaginary parts, which is what JuMP is already doing.

As for 2), isn't the ScaledPSDCone in MOI defined only for real symmetric matrices? Also, why would I want the version scaled by sqrt(2), since SeDuMi wants the non-scaled version as input?

@blegat
Copy link
Member Author

blegat commented May 23, 2023

Isn't 1) just allowing complex numbers in the equality constraints? But what is the benefit of doing that directly in SeDuMi? What SeDuMi will do is just split the constraint into real and imaginary parts, which is what JuMP is already doing.

Not sure what SeDuMi does internally but if it just reformulates that might not be too interesting, you can just do 2) if you're only interested in that.

As for 2), isn't the ScaledPSDCone in MOI defined only for real symmetric matrices? Also, why would I want the version scaled by sqrt(2), since SeDuMi wants the non-scaled version as input?

SeDuMi.ScaledPSDCone is different from MOI.ScaledPositiveSemidefiniteConeTriangle. It seems that SeDuMi needs the matrix in square form (so the n^2 entries) and I think you can either give the same entries twice, once for each off-diagonal entry or give it once and multiply the off-diagonal entries by 2.
So

x 2y
0 z

is the same as

x y
y z

That's what I remember from when I wrote the SeDuMi wrapper. All Matlab SDP solver (SDPT3, SeDuMi, CDCS, SDPNAL, ...) are quite different in the way they want the matrix to be inputed so it was a bit of trial and error until the MOI tests pass and I don't remember which one wanted what exactly. I'm quite confident that scaling by 2 works in the real cases so I assume it should work as well in the complex case.

@araujoms
Copy link
Contributor

Ok, I see, SeDuMi.ScaledPSDCone is just how you defined the input for SeDuMi specifically, it has nothing to do with the MOI cones. That's why you wanted a bridge from the MOI cone (which is what JuMP will produce) to it.

SeDuMi does definitely want the matrix in square form as input. I also know that internally it will discard the upper triangular and re-scale the lower triangular. I would assume that doing that on the input would wreak havoc, but I guess it will become obvious why it works as I start tinkering. Thanks again!

@blegat
Copy link
Member Author

blegat commented May 23, 2023

Yes, we want to do everything as bridge so that it works well with MatrixOfConstraints and we can just take the matrix as is

@araujoms
Copy link
Contributor

I got stuck. I'm trying to promote the VectorAffineTerm to complex variables, but MOI instead insists in mapping them back to reals. Any ideas? Specifically I'm trying to do
x[i] = MOI.VectorAffineTerm(map[x[i].output_index-triangle_size], MOI.ScalarAffineTerm(ComplexF64(im) * ComplexF64(x[i].scalar_term.coefficient), x[i].scalar_term.variable))
but I get the error InexactError: Float64(0.0 + 1.0im), because x is a Vector{MathOptInterface.VectorAffineTerm{Float64}} and I can't find a way to promote it to Vector{MathOptInterface.VectorAffineTerm{ComplexF64}}.

@blegat
Copy link
Member Author

blegat commented May 25, 2023

convert(Vector{MathOptInterface.VectorAffineTerm{ComplexF64}}, x) ?

@araujoms
Copy link
Contributor

That works, thanks. I thought that was equivalent to ComplexF64.(x), which doesn't work, so I didn't even try.

@araujoms
Copy link
Contributor

I'm stuck again. I can't convince MOI that SeDuMi now does support MathOptInterface.VectorAffineFunction{ComplexF64}-in-SeDuMi.ScaledPSDCone, even though I did add

function MOI.supports_constraint(
    ::Optimizer,
    ::Type{MOI.VectorAffineFunction{ComplexF64}},
    ::Type{ScaledPSDCone}
)
    return true
end

I suppose I need something like the boilerplate you wrote for product_of_sets, struct_of_constraints_by_function_types, and OptimizerCache, but that doesn't compile because of syntax errors, and I don't know how to fix that.

@blegat
Copy link
Member Author

blegat commented May 26, 2023

Could you show me the code and the syntax error ? Maybe it's easier if you open a PR with your current code and show the error in the comment of the PR. I added you write access so that you can create a branch on the upstream remote directly so that I can modify it as well

@araujoms
Copy link
Contributor

Well the syntax errors are caused by your code alone, you can literally paste that into MOI_wrapper.jl and see them. Some I could fix, but not all.

@araujoms
Copy link
Contributor

Thanks for giving me write access, but accepting it requires me to enable two-factor authentication, which I refuse. I can do a regular PR instead if you want.

@odow
Copy link
Member

odow commented May 26, 2023

Thanks for giving me write access, but accepting it requires me to enable two-factor authentication, which I refuse

Any reason? This is a standard security policy for all repositories in jump-dev. Note that you don't have to use a phone number as the 2FA. See https://docs.github.com/en/authentication/securing-your-account-with-two-factor-authentication-2fa/configuring-two-factor-authentication

@araujoms
Copy link
Contributor

Because 2FA is a huge pain in the ass, I'd rather keep using just the password to login.

@odow
Copy link
Member

odow commented May 27, 2023

Because 2FA is a huge pain in the ass, I'd rather keep using just the password to login

From our perspective, 2FA is to minimize the risk of someone's account being compromised and used to push malicious code to one of the repo's in JuMP-dev.

But also, the 2FA is pretty minimal. I stay logged into my account on my computer, so the only time I ever need to validate is performing an admin permission, like moving a repository, or changing secrets. You don't need to validate every time you open a PR or commit, so it shouldn't impact your day-to-day workflow.

@araujoms
Copy link
Contributor

It is out of the question, I'm not going to mess up my github account, specially not in order to contribute to somebody else's project.

Moreover, the idea that someone would be interested in pushing malicious code to jump-dev repositories strikes me as paranoid. Such a commit would be instantly reverted and never become part of a release. And if you think my account is insecure you're welcome to try and hack it.

@blegat
Copy link
Member Author

blegat commented May 28, 2023

I added you write access so that you can create a branch on the upstream remote directly so that I can modify it as well

It's fine if you push to your fork and create a PR, forget about it ;)

Some I could fix, but not all.

Since you could fix some, I'd rather start from your branch than from my untested code

@araujoms
Copy link
Contributor

I've already submitted a PR. And I did check the box "Allow edits by maintainers", so you have write access to it.

@blegat
Copy link
Member Author

blegat commented May 28, 2023

Yes, I missed that thanks, I'm going to check next week

@blegat blegat linked a pull request May 30, 2023 that will close this issue
@blegat
Copy link
Member Author

blegat commented May 30, 2023

Just pushed to your branch, it seems to go through, you should have what you need in Ac_complex now

@araujoms
Copy link
Contributor

Awesome, thanks, indeed the data now gets to where it needs to be.

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

Successfully merging a pull request may close this issue.

3 participants