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

function_string of Symmetric matrix includes the entire matrix #3764

Closed
LebedevRI opened this issue May 31, 2024 · 7 comments · Fixed by #3768
Closed

function_string of Symmetric matrix includes the entire matrix #3764

LebedevRI opened this issue May 31, 2024 · 7 comments · Fixed by #3768
Labels
Category: Printing Related to printing

Comments

@LebedevRI
Copy link
Contributor

LebedevRI commented May 31, 2024

using JuMP
import LinearAlgebra
import MathOptInterface as MOI

function main()
    model = Model() 

    @variable(model, v[1:3,1:3], Symmetric)

    @constraint(model, c0, LinearAlgebra.Symmetric(v.-1)  == LinearAlgebra.Symmetric(fill(41, 3,3))) # Yay!
    @show(c0) # Uhh, why is it not symmetric?
    print(c0) # Uhh, why is it not symmetric?
    display(c0) # Oh, but it is symmetric here.
    print(model) # And it is symmetric here too.
end

main()
c0 = c0 : [v[1,1] - 42  v[1,2] - 42  v[1,3] - 42;
 v[1,2] - 42  v[2,2] - 42  v[2,3] - 42; ∈ Zeros()
 v[1,3] - 42  v[2,3] - 42  v[3,3] - 42]

image
This is even worse for text-only output, looks like only pretty-printing does it right.

@odow
Copy link
Member

odow commented May 31, 2024

text/latex has a special check for Symmetric. We don't have the check in text/plain. But that's also because we defer the printing to Base.show, so I'm not sure this is easy to fix.

JuMP.jl/src/print.jl

Lines 884 to 919 in 56b186f

function function_string(
::MIME"text/plain",
A::AbstractMatrix{<:AbstractJuMPScalar},
)
str = sprint() do io
return show(IOContext(io, :limit => true), MIME("text/plain"), A)
end
lines = split(str, '\n')
# We drop the first line with the signature "m×n Array{...}:"
popfirst!(lines)
# We replace the first space by an opening `[`
lines[1] = '[' * lines[1][2:end]
return join(lines, "\n") * "]"
end
function function_string(
mode::MIME"text/latex",
A::AbstractMatrix{<:AbstractJuMPScalar},
)
str = "\\begin{bmatrix}\n"
for i in axes(A, 1)
line = ""
for j in axes(A, 2)
if j != 1
line *= " & "
end
if A isa LinearAlgebra.Symmetric && i > j
line *= "\\cdot"
else
line *= function_string(mode, A[i, j])
end
end
str *= line * "\\\\\n"
end
return str * "\\end{bmatrix}"
end

In the REPL, Julia prints the full symmetric matrix:

julia> using LinearAlgebra

julia> Symmetric([1 2; 2 3])
2×2 Symmetric{Int64, Matrix{Int64}}:
 1  2
 2  3

@odow odow added the Category: Printing Related to printing label May 31, 2024
@LebedevRI
Copy link
Contributor Author

In the REPL, Julia prints the full symmetric matrix:

Right, i just saw that too. I thought it didn't, but apparently it does.

@odow
Copy link
Member

odow commented May 31, 2024

Arguably someone might interpret out current text/latex printing as the upper triangular, with zeros in the lower.

Here's what UpperTriangular prints.

julia> UpperTriangular([1 2; 2 3])
2×2 UpperTriangular{Int64, Matrix{Int64}}:
 1  2
   3

I don't even know if the current text/latex approach is a good idea.

@odow odow changed the title Textually print()ing (instead of display()) of symmetric constraint shows the entire matrix function_string of Symmetric matrix includes the entire matrix May 31, 2024
@LebedevRI
Copy link
Contributor Author

IMHO, the text/latex approach is generally the right way to do it. Perhaps a different symbol should be used?

@odow
Copy link
Member

odow commented Jun 3, 2024

Thoughts @blegat? I would err on the side of sticking to Base.

@odow
Copy link
Member

odow commented Jun 4, 2024

See #3768

@odow odow closed this as completed in #3768 Jun 5, 2024
@LebedevRI
Copy link
Contributor Author

@odow thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Printing Related to printing
Development

Successfully merging a pull request may close this issue.

2 participants