Performance/style improvement with buffer printing?

[gustaphe]

When working on the complex and rational numbers, I got thinking about how this package is passing strings around. From what I've learned in other languages, building strings by concatenating other strings has a lot more overhead than making a buffer and printing to it. One way to refactor this package would be to introduce

function latexify(x)
    io = IOBuffer();
    print(io,'$')
    show(io,x,MIME"text/latexify"())
    print(io,'$')
    return String(take!(io))
end

function show(io::IOBuffer,x::Any,mime::MIME"text/latexify")
    show(io,x,MIME"text/plain")
end

function show(io::IOBuffer,x::Rational,mime::MIME"text/latexify")
    print(io,"\frac{")
    show(io,numerator(x),mime)
    print(io,"}{")
    show(io,denominator(x),mime)
    print(io,'}')
end

etc. Is there a strong reason to not do this? It should be pretty straightforward to switch from building a string inside every function to printing to the same buffer each time. I could give it a try, just want to check that it hasn't been tried and discarded.

[korsbo]

I'm not opposed to this in principle but my initial guess is that changing this would be a bit more complicated than you seem to think.

We'd still need to actually return a LaTeXString, not just invoke show. Otherwise, latexify could not be used to produce things such as labels for plots, etc.

Also, some operations work directly on the strings by replace (e.g. the unicode bit).

When I started making this package, my fingers were itching to optimise it to perfection but I refrained since I did not think it worthwhile. Have you found latexify to affect performance in any way? Even if the string manipulation is slow, I've never used latexify on anything large enough for execution time to be noticeable. I have thus valued code readability and code configurability over computational performance.

But if you think that doing this would provide value to you or anyone else, and if it does not make the code harder to maintain, then feel free to tinker!

[korsbo]

Now is a good time to revisit this since I'm refactoring anyways.

I just spent 15 minutes formulating all the ways in which this would be hard only to find myself able to poke holes in all my arguments. Maybe this would be good.

If it could improve compile time then I'd be very happy. I'm pretty sure that it could improve runtime but I don't think that's an issue since latexify already has pretty much no notable runtime.

[korsbo]

hmmm, join and friends could still be difficult.

if every step in the recursion writes to io rather than returns a string then stuff like

join(io, my_matrix, " &= ")

would work but not when you also wanted to descend into my_matrix and latexify its elements.

join(io, descend.(io, my_matrix), " &= ")

would not work since descend no longer returns a string but instead writes to IO. The order would be off since descend is called before join.

[korsbo]

nevermind. Something like

function join_descend(io, args, delim) 
  for arg in args[1:end-1]
    decend(io, arg)
    write(io, delim)
  end
  decend(io, arg[end])
end

should just work as a drop-in replacement for join where I also want to descend the arguments.

[gustaphe]

I have no idea how this would affect compile time, and your argument about runtime is a strong one. But I still think this is principally a good idea.

Perhaps working with Strings is unavoidable when it comes to the unicode conversion. But you'll only do that on string and symbol arguments anyway.

One really nice thing about replacing (or supplementing) descend(x) with show(io::IO, mime::MIME"text/latexify", x) is that packages could implement show(io::IO, mime::MIME"text/latexify", x::MyType) without importing anything from Latexify, as long as they don't need to affect the top-level parameters like default environment etc. Parameters (e.g. displaymode/textmode) could be passed around using IOContext.

[korsbo]

One really nice thing about replacing (or supplementing) descend(x) with show(io::IO, mime::MIME"text/latexify", x) is that packages could implement show(io::IO, mime::MIME"text/latexify", x::MyType) without importing anything from Latexify, as long as they don't need to affect the top-level parameters like default environment etc. Parameters (e.g. displaymode/textmode) could be passed around using IOContext.

huh, that's a good point. It's a very different approach to what I'm working with now where I'm using an array (that can be push!ed) of functions that are used for the conversion. Each function first checks if it's applicable (if not, goto next function in the array) and then applies their formatting. Users can thus push new functions to that array.

Doing it with show and a custom MIME type might also work. Although instead of being able to use any conditional one would be stuck with only being able to use method dispatch to determine how to do formatting.

I really like the idea that extending packages would then not even need to import latexify.jl though.

[gustaphe]

Sorry to throw extra stuff in the mix.

instead of being able to use any conditional one would be stuck with only being able to use method dispatch to determine how to do formatting.

Could you give an example of what kind of formatting needs this / what type of conditional? I'm sure many things can be solved by passing things through IOContext:

function show(io, mime::MIME"text/latexify", x::Multiplication)
    surround = compare_precedence(get(io, :precedence, :+), :*)
    io = IOContext(io, :precedence=>:*)
    surround ? write(io, "\\left(") : nothing
    show(io, mime, x.a)
    write(io, get(io, :multsign, "\\cdot"))
    show(io, mime, x.b)
    surround ? write(io, "\\right)") : nothing
end

That said, like I said, I don't know if there are any big benefits to this way over the array of functions. If you want I could formulate a mockup in the next couple of days.

[korsbo]

This is interesting. Untimely but interesting. I need a solution for the compile time before Thursday when there's a feature freeze on Pumas.jl. I have been working that in to the refactor that I'm working on. That refactor is fairly mature so I should be able to get that done in time. Switching to this approach might mess that up.

That said. I do find the approach interesting and worth exploring.

[korsbo]

The following works. Any suggestions for improvements?

abstract type AbstractLatexifyOperation end
struct Multiplication{T} <: AbstractLatexifyOperation
  args::T
end

struct Subtraction{T} <: AbstractLatexifyOperation
  args::T
end

struct Addition{T} <: AbstractLatexifyOperation
  args::T
end

struct Division{T} <: AbstractLatexifyOperation
  args::T
end

latexop(ex::Expr) = latexop(Val(ex.head), Val(ex.args[1]), ex.args[2:end])
latexop(x) = x

latexop(::Val{:call}, ::Val{:*}, args) = Multiplication(args)
latexop(::Val{:call}, ::Val{:+}, args) = Addition(args)
latexop(::Val{:call}, ::Val{:/}, args) = Division(args)

show(io::IO, ::MIME"text/latexify", x) = print(io, x)

compare_precedence(a, b) = Base.operator_precedence(a) > Base.operator_precedence(b)
function show(io::IO, mime::MIME"text/latexify", x::Multiplication)
    surround = compare_precedence(get(io, :precedence, :+), :*)
    io = IOContext(io, :precedence=>:*)
    surround ? write(io, "\\left( ") : nothing
    show(io, mime, latexop(x.args[1]))
    write(io, get(io, :multsign, " \\cdot "))
    show(io, mime, latexop(x.args[2]))
    surround ? write(io, "\\right) ") : nothing
end

function show(io::IO, mime::MIME"text/latexify", x::Addition)
    surround = compare_precedence(get(io, :precedence, :+), :+)
    io = IOContext(io, :precedence=>:+)
    surround ? write(io, "\\left(") : nothing
    for arg in x.args[1:end-1]
        show(io, mime, latexop(arg))
        write(io, " + ")
    end
    show(io, mime, latexop(x.args[end]))
    surround ? write(io, "\\right)") : nothing
end

function show(io::IO, mime::MIME"text/latexify", x::Division)
    write(io, "\\frac{")
    show(io, mime, latexop(x.args[1]))
    write(io, "}{")
    show(io, mime, latexop(x.args[2]))
    write(io, "}")
end

function show(io::IO, mime::MIME"text/latexify", x::Expr)
    show(io, mime, latexop(x))
end

ex = :(x*(y+b)/(c+2))
io = IOBuffer(;append=true)
io = IOContext(io, :multsign => " \\times ")
show(io, MIME("text/latexify"), ex)
read(io, String)