Remove dependency on MutableArithmetics

[odow]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.16%. Comparing base (7a6130d) to head (4872191).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...c/MOI_wrapper/hermitian_complex_psd_cone_bridge.jl	0.00%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #330   +/-   ##
=======================================
  Coverage   92.16%   92.16%           
=======================================
  Files          12       12           
  Lines         689      689           
=======================================
  Hits          635      635           
  Misses         54       54           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[araujoms]

Why did you do this? I get a brutal slowdown, 0.120101 seconds (106.54 k allocations: 423.958 MiB, 38.21% gc time) versus 0.001079 seconds (19.64 k allocations: 1.191 MiB) for an inner product between 50 x 50 complex Hermitian matrices, and I don't see the benefit, since MutableArithmetics is still a dependency through MathOptInterface.

[odow]

Do you have a reproducible example?

[araujoms]

Before your change

julia> using JuMP, SCS

julia> model = Model();

julia> set = SCS.ComplexPositiveSemidefiniteConeTriangle(50);

julia> @variable(model, x[1:2500]);

julia> @variable(model, y[1:2500]);

julia> @time MOI.Utilities.set_dot(x,y,set);
  0.370305 seconds (2.31 M allocations: 113.229 MiB, 4.22% gc time, 99.87% compilation time)

julia> @time MOI.Utilities.set_dot(x,y,set);
  0.000867 seconds (19.64 k allocations: 1.191 MiB)

After your change:

julia> using JuMP, SCS
Precompiling SCS finished.
  1 dependency successfully precompiled in 22 seconds. 56 already precompiled.

julia> model = Model();

julia> set = SCS.ComplexPositiveSemidefiniteConeTriangle(50);

julia> @variable(model, x[1:2500]);

julia> @variable(model, y[1:2500]);

julia> @time MOI.Utilities.set_dot(x,y,set);
  0.295810 seconds (291.24 k allocations: 431.760 MiB, 45.06% gc time, 39.39% compilation time)

julia> @time MOI.Utilities.set_dot(x,y,set);
  0.111543 seconds (106.55 k allocations: 422.962 MiB, 43.15% gc time)

[odow]

What about an example where it isn't called directly? You're passing in JuMP variables. In MOI, it's only really used to compute the dual objective value, and it will always have Float64 as the input type.

I'd be willing to just lock in many of the T types in the bridges for Float64. We're not trying to be generic. These sets and bridges have a very specific purpose.

[araujoms]

No, I'm not going to waste my time writing more examples. Perhaps you shouldn't change code around for no benefit? I wrote the function the way it's done in MOI. And it will certainly be used for numerical types other than Float64, there are several solvers which can handle BigFloat.

[odow]

My point was that, can you actually trigger this when using SCS.Optimizer? I don't think so.

And it will certainly be used for numerical types other than Float64, there are several solvers which can handle BigFloat.

But this bridge in SCS is not one of them?

I dislike Julia code that is overly generic. @blegat can chime in.

[odow]

These functions aren't even covered by the tests:

https://app.codecov.io/gh/jump-dev/SCS.jl/blob/master/src%2FMOI_wrapper%2Fsets%2FComplexPositiveSemidefiniteConeTriangle.jl

[araujoms]

The point of writing this set was to incorporate it into MOI as the replacement for the complex PSD cone, as suggested by @blegat. It's supposed to be generic.

[odow]

We already have too many PSD cones in MOI. I don't want to add another. It can stay in SCS.jl.

[araujoms]

The point is not to add another set, but to replace one.

[blegat]

My rule of thumb is: if a set is used by only one solver then the set and bridges should be defined by the solver. When it's used by several ones, we can move it to MOI. So at the moment it is in SCS but we may move it in the future. If it's moved in the future, the MA calls will be more appropriate, e.g. add_mul!! will be faster with BigFloat. But since SCS doesn't support BigFloat then both are fine. Since MA was already a dependency, maybe we could have just kept it but I think here both are equally fine so no point debating. Maybe we can just add a comment in the code pointing to this PR saying that we should consider switching back to the more generic version in case we move it to MOI.

[araujoms]

Hypatia already uses this set, so if we move it to MOI we can remove Hypatia's bridge.

[blegat]

Indeed, I guess it would allow getting rid of
https://github.com/jump-dev/Hypatia.jl/blob/master/src/MathOptInterface/transform.jl#L159-L214
We could even get rid of the rescaling of Hypatia
https://github.com/jump-dev/Hypatia.jl/blob/master/src/MathOptInterface/transform.jl#L73-L89
if we just say that Hypatia needs Scaled{ComplexPSDTriangle}
But before this, I think the first step is for Hypatia to use the MOI.Scaled set to simplify
https://github.com/jump-dev/Hypatia.jl/blob/master/src/MathOptInterface/transform.jl#L51-L57

[araujoms]

Fair enough, I added MOI.Scaled to Hypatia.