Add TranslateVarArray for simplified indexing

[hellemo]

There is a considerable overhead when indexing on custom types. This PR investigates if we can relatively easily translate to a simpler and type stable value for indexing under the hood:

using BenchmarkTools
using JuMP
using SparseVariables

const SV = SparseVariables

# Demo custom types
abstract type Node end
struct Source <: Node
    id::Symbol
end
Source(n::Int) = Source(Symbol(string(n)))
struct Sink <: Node
    id::Symbol
end
Sink(n::Int) = Sink(Symbol(string(n)))

# Translation to type-stable index
@inline SV.translate(n::Node) = n.id
@inline SV.translate(::Type{<:Node}) = Symbol

"""
    Indexing on custom types with `IndexedVarArray`
"""
function test(N)
    m = Model()
    @variable(
        m,
        x[i = Sink.(1:N), y = Source.(1:N)],
        container = SV.IndexedVarArray
    )
    for i in Sink.(1:N), j in Source.(1:N)
        insertvar!(x, i, j)
    end
end

"""
    Indexing on `Int` manually for performance with `IndexedVarArray`
"""
function test_ix(N)
    m = Model()
    @variable(m, x[i = 1:N, j = 1:N], container = SV.IndexedVarArray)
    for i = 1:N, j = 1:N
        insertvar!(x, i, j)
    end
end

"""
    Automatic translation to `Int` by `TranslateVarArray`
"""
function test_t(N)
    m = Model()
    @variable(
        m,
        x[i = Sink.(1:N), j = Source.(1:N)],
        container = SV.TranslateVarArray
    )
    for i in Sink.(1:N), j in Source.(1:N)
        insertvar!(x, i, j)
    end
end

"""
    Manual indexing by `Int` with `TranslateVarArray` for comparison
"""
function test_i(N)
    m = Model()
    @variable(m, x[i = 1:N, j = 1:N], container = SV.TranslateVarArray)
    for i = 1:N, j = 1:N
        insertvar!(x, i, j)
    end
end


N = 100
@info "Custom type w/IndexedVarArray"
@btime test(N)

@info "Manual Int w/IndexedVarArray"
@btime test_ix(N)

@info "Automatic w/TranslateVarArray"
@btime test_t(N)

@info "Manual Int w/TranslateVarArray"
@btime test_i(N)

julia> include("translate_benchmark.jl")
[ Info: Custom type w/IndexedVarArray
  68.888 ms (560928 allocations: 24.09 MiB)
[ Info: Manual Int w/IndexedVarArray
  9.397 ms (200227 allocations: 11.38 MiB)
[ Info: Automatic w/TranslateVarArray
  22.143 ms (290928 allocations: 13.25 MiB)
[ Info: Manual Int w/TranslateVarArray
  12.727 ms (230227 allocations: 12.29 MiB)

[codecov-commenter]

Codecov Report

Patch coverage: 93.82% and project coverage change: +1.05 🎉

Comparison is base (a8afb3e) 88.25% compared to head (220d31f) 89.31%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   88.25%   89.31%   +1.05%     
==========================================
  Files           5        6       +1     
  Lines         315      393      +78     
==========================================
+ Hits          278      351      +73     
- Misses         37       42       +5     

Impacted Files	Coverage Δ
src/SparseVariables.jl	100.00% <ø> (ø)
src/translate.jl	93.58% <93.58%> (ø)
src/tables.jl	63.15% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[trulsf]

Nice with higher efficiency! Should the new container just replace the existing IndexedVarArray? Or is it ok with two different types for a while?
Tables-support is not working for TranslateVarArray, but should be easy to fix.

[hellemo]

Looks like I misplaced a comment somewhere.. just added support for Tables.

I think we could use this as the default, but there is some overhead with the translation which I would like to remove/reduce. Also a TranslateVarArray does not store the structs, so if anyone use the container to lookup the keys, that will break (but not a part of the API).

Is it ok to keep as a separate type for now and see how it works for larger models and maybe cut down on the overhead? And then do a larger API overhaul for the next breaking release? There is a lot of overlap in the code between the two types, so a cleanup would be nice if we like this interface.

Not super happy about the naming, though, any suggestions? (Both for TranslateVarArray and translate)

[trulsf]

I am fine with separate types for now. Not sure about better names... MappedIndexedVarArray with function map_to_index ?