From 6cccc30036b4f93b8882efaa2597d4c1439c070b Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Mon, 24 May 2021 11:05:24 +1200
Subject: [PATCH 01/11] WIP: fix variety of bugs in ProductOfSets

---
 src/Utilities/product_of_sets.jl  |  84 ++++++------
 test/Utilities/product_of_sets.jl | 219 ++++++++++++++++++++++++++++++
 2 files changed, 261 insertions(+), 42 deletions(-)
 create mode 100644 test/Utilities/product_of_sets.jl

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index c9923755e5..5d8b0bc456 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -12,7 +12,7 @@ Return an integer corresponding to the index of the set type in the list given
 by [`set_types`](@ref). If this set is not part of the list then it returns
 `nothing`.
 """
-set_index(sets::ProductOfSets, ::Type{S}) where {S<:MOI.AbstractSet} = nothing
+set_index(::ProductOfSets, ::Type{S}) where {S<:MOI.AbstractSet} = nothing
 
 """
     set_types(sets::ProductOfSets)
@@ -73,6 +73,7 @@ Product of scalar sets in the order the constraints are added, mixing the
 constraints of different types.
 """
 abstract type MixOfScalarSets{T} <: ProductOfSets{T} end
+
 macro mix_of_scalar_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
@@ -86,13 +87,18 @@ macro mix_of_scalar_sets(name, set_types...)
 end
 
 MOI.is_empty(sets::MixOfScalarSets) = isempty(sets.set_ids)
+
 MOI.empty!(sets::MixOfScalarSets) = empty!(sets.set_ids)
+
 MOI.dimension(sets::MixOfScalarSets) = length(sets.set_ids)
+
 indices(::MixOfScalarSets, ci::MOI.ConstraintIndex) = ci.value
+
 function add_set(sets::MixOfScalarSets, i)
     push!(sets.set_ids, i)
     return length(sets.set_ids)
 end
+
 function MOI.get(
     sets::MixOfScalarSets{T},
     ::MOI.ListOfConstraintTypesPresent,
@@ -103,6 +109,7 @@ function MOI.get(
         S in set_types(sets) if set_index(sets, S) in present
     ]
 end
+
 function MOI.get(
     sets::MixOfScalarSets,
     ::MOI.NumberOfConstraints{F,S},
@@ -110,19 +117,18 @@ function MOI.get(
     i = set_index(sets, S)
     return count(isequal(i), sets.set_ids)
 end
+
 function MOI.get(
     sets::MixOfScalarSets,
     ::MOI.ListOfConstraintIndices{F,S},
 ) where {F,S}
     i = set_index(sets, S)
-    return LazyMap{MOI.ConstraintIndex{F,S}}(
-        j -> MOI.ConstraintIndex{F,S}(j),
-        Base.Iterators.Filter(
-            j -> sets.set_ids[j] == i,
-            eachindex(sets.set_ids),
-        ),
-    )
+    return MOI.ConstraintIndex{F,S}[
+        MOI.ConstraintIndex{F,S}(j)
+        for j in eachindex(sets.set_ids) if sets.set_ids[j] == i
+    ]
 end
+
 function MOI.is_valid(
     sets::MixOfScalarSets,
     ci::MOI.ConstraintIndex{F,S},
@@ -140,6 +146,7 @@ Product of sets in the order the constraints are added, grouping the
 constraints of the same types contiguously.
 """
 abstract type OrderedProductOfSets{T} <: ProductOfSets{T} end
+
 macro product_of_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
@@ -155,35 +162,13 @@ macro product_of_sets(name, set_types...)
     return _sets_code(esc_name, T, type_def, set_types...)
 end
 
-"""
-    abstract type OrderedProductOfScalarSets{T} <: OrderedProductOfSets{T} end
-
-Same as [`OrderedProductOfSets`](@ref) except that all types are scalar sets,
-which allows a more efficient implementation.
-"""
-abstract type OrderedProductOfScalarSets{T} <: OrderedProductOfSets{T} end
-macro product_of_scalar_sets(name, set_types...)
-    esc_name = esc(name)
-    T = esc(:T)
-    type_def = :(struct $esc_name{$T} <: $MOIU.OrderedProductOfScalarSets{$T}
-        num_rows::Vector{Int}
-        function $esc_name{$T}() where {$T}
-            return new(zeros(Int, $(1 + length(set_types))))
-        end
-    end)
-    return _sets_code(esc_name, T, type_def, set_types...)
-end
-
 MOI.is_empty(sets::OrderedProductOfSets) = all(iszero, sets.num_rows)
+
 function MOI.empty!(sets::OrderedProductOfSets)
     fill!(sets.num_rows, 0)
     empty!(sets.dimension)
     return
 end
-function MOI.empty!(sets::OrderedProductOfScalarSets)
-    fill!(sets.num_rows, 0)
-    return
-end
 
 function MOI.dimension(sets::OrderedProductOfSets)
     for i in 3:length(sets.num_rows)
@@ -191,6 +176,7 @@ function MOI.dimension(sets::OrderedProductOfSets)
     end
     return sets.num_rows[end]
 end
+
 function indices(
     sets::OrderedProductOfSets{T},
     ci::MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},S},
@@ -198,6 +184,7 @@ function indices(
     i = set_index(sets, S)
     return sets.num_rows[i] + ci.value + 1
 end
+
 function indices(
     sets::OrderedProductOfSets{T},
     ci::MOI.ConstraintIndex{MOI.VectorAffineFunction{T},S},
@@ -205,21 +192,25 @@ function indices(
     i = set_index(sets, S)
     return (sets.num_rows[i] + ci.value) .+ (1:sets.dimension[(i, ci.value)])
 end
+
 function add_set(sets::OrderedProductOfSets, i)
     offset = sets.num_rows[i+1]
     sets.num_rows[i+1] = offset + 1
     return offset
 end
+
 function add_set(sets::OrderedProductOfSets, i, dim)
     offset = sets.num_rows[i+1]
     sets.num_rows[i+1] = offset + dim
     sets.dimension[(i, offset)] = dim
     return offset
 end
+
 function _num_indices(sets::OrderedProductOfSets, ::Type{S}) where {S}
     i = set_index(sets, S)
     return sets.num_rows[i+1] - sets.num_rows[i]
 end
+
 function MOI.get(
     sets::OrderedProductOfSets{T},
     ::MOI.ListOfConstraintTypesPresent,
@@ -229,23 +220,28 @@ function MOI.get(
         S in set_types(sets) if !iszero(_num_indices(sets, S))
     ]
 end
-struct UnevenIterator
+
+struct _UnevenIterator
     i::Int
     start::Int
     stop::Int
     dimension::Dict{Tuple{Int,Int},Int}
 end
-Base.IteratorSize(::UnevenIterator) = Base.SizeUnknown()
-function Base.iterate(it::UnevenIterator, cur = it.start)
+
+Base.IteratorSize(::_UnevenIterator) = Base.SizeUnknown()
+
+function Base.iterate(it::_UnevenIterator, cur = it.start)
     if cur >= it.stop
         return nothing
     else
         return (cur, cur + it.dimension[(it.i, cur)])
     end
 end
-function Base.in(x, it::UnevenIterator)
+
+function Base.in(x, it::_UnevenIterator)
     return x in it.start:(it.stop-1) && haskey(it.dimension, (it.i, x))
 end
+
 function _range_iterator(
     ::OrderedProductOfSets{T},
     ::Int,
@@ -255,6 +251,7 @@ function _range_iterator(
 ) where {T}
     return start:(stop-1)
 end
+
 function _range_iterator(
     sets::OrderedProductOfSets{T},
     i::Int,
@@ -262,8 +259,9 @@ function _range_iterator(
     stop::Int,
     ::Type{MOI.VectorAffineFunction{T}},
 ) where {T}
-    return UnevenIterator(i, start, stop, sets.dimension)
+    return _UnevenIterator(i, start, stop, sets.dimension)
 end
+
 function _range(
     sets::OrderedProductOfSets{T},
     ::Type{F},
@@ -282,8 +280,10 @@ function _range(
         )
     end
 end
+
 _length(r::UnitRange) = length(r)
-_length(r::UnevenIterator) = count(_ -> true, r)
+_length(r::_UnevenIterator) = count(_ -> true, r)
+
 function MOI.get(
     sets::OrderedProductOfSets,
     ::MOI.NumberOfConstraints{F,S},
@@ -295,17 +295,19 @@ function MOI.get(
         return _length(r)
     end
 end
+
 function _empty(
     ::OrderedProductOfSets{T},
     ::Type{<:MOI.ScalarAffineFunction},
 ) where {T}
     return 1:0
 end
+
 function _empty(
     sets::OrderedProductOfSets{T},
     ::Type{<:MOI.VectorAffineFunction},
 ) where {T}
-    return UnevenIterator(1, 1, 0, sets.dimension)
+    return _UnevenIterator(1, 1, 0, sets.dimension)
 end
 
 function MOI.get(
@@ -317,11 +319,9 @@ function MOI.get(
         # Empty iterator
         rows = _empty(sets, F)
     end
-    return LazyMap{MOI.ConstraintIndex{F,S}}(
-        i -> MOI.ConstraintIndex{F,S}(i),
-        rows,
-    )
+    return MOI.ConstraintIndex{F,S}.(rows)
 end
+
 function MOI.is_valid(
     sets::OrderedProductOfSets,
     ci::MOI.ConstraintIndex{F,S},
diff --git a/test/Utilities/product_of_sets.jl b/test/Utilities/product_of_sets.jl
new file mode 100644
index 0000000000..db4e3983b5
--- /dev/null
+++ b/test/Utilities/product_of_sets.jl
@@ -0,0 +1,219 @@
+module TestProductOfSets
+
+using Test
+
+import MathOptInterface
+const MOI = MathOptInterface
+
+function runtests()
+    for name in names(@__MODULE__; all = true)
+        if startswith("$(name)", "test_")
+            @testset "$(name)" begin
+                getfield(@__MODULE__, name)()
+            end
+        end
+    end
+    return
+end
+
+MOI.Utilities.@mix_of_scalar_sets(
+    _ScalarSets,
+    MOI.EqualTo{T},
+    MOI.GreaterThan{T},
+    MOI.LessThan{T},
+    MOI.Interval{T},
+)
+
+function test_scalar_set_index()
+    sets = _ScalarSets{Float64}()
+    @test MOI.Utilities.set_index(sets, MOI.EqualTo{Float64}) == 1
+    @test MOI.Utilities.set_index(sets, MOI.GreaterThan{Float64}) == 2
+    @test MOI.Utilities.set_index(sets, MOI.Interval{Float64}) == 4
+    @test MOI.Utilities.set_index(sets, MOI.LessThan{Float64}) == 3
+    @test MOI.Utilities.set_index(sets, MOI.ZeroOne) === nothing
+end
+
+function test_scalar_set_types()
+    sets = _ScalarSets{Float64}()
+    @test MOI.Utilities.set_types(sets) == [
+        MOI.EqualTo{Float64},
+        MOI.GreaterThan{Float64},
+        MOI.LessThan{Float64},
+        MOI.Interval{Float64},
+    ]
+end
+
+function test_scalar_basic()
+    sets = _ScalarSets{Float64}()
+    ci = MOI.ConstraintIndex{
+        MOI.ScalarAffineFunction{Float64},
+        MOI.EqualTo{Float64},
+    }(12345)
+    @test !MOI.is_valid(sets, ci)
+    i = MOI.Utilities.set_index(sets, MOI.EqualTo{Float64})
+    ci_value = MOI.Utilities.add_set(sets, i)
+    ci = MOI.ConstraintIndex{
+        MOI.ScalarAffineFunction{Float64},
+        MOI.EqualTo{Float64},
+    }(ci_value)
+    @test MOI.is_valid(sets, ci)
+    @test MOI.Utilities.indices(sets, ci) == ci.value
+end
+
+function test_scalar_dimension()
+    sets = _ScalarSets{Float64}()
+    @test MOI.dimension(sets) == 0
+    MOI.Utilities.add_set(sets, 1)
+    @test MOI.dimension(sets) == 1
+    MOI.Utilities.add_set(sets, 1)
+    @test MOI.dimension(sets) == 2
+    MOI.Utilities.add_set(sets, 2)
+    @test MOI.dimension(sets) == 3
+end
+
+function test_scalar_empty()
+    sets = _ScalarSets{Float64}()
+    @test MOI.is_empty(sets)
+    MOI.Utilities.add_set(sets, 1)
+    @test !MOI.is_empty(sets)
+    MOI.empty!(sets)
+    @test MOI.is_empty(sets)
+end
+
+function test_scalar_ConstraintTypesPresent()
+    sets = _ScalarSets{Float64}()
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
+    MOI.Utilities.add_set(sets, 1)
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.ScalarAffineFunction{Float64},MOI.EqualTo{Float64})]
+end
+
+function test_scalar_NumberOfConstraints()
+    sets = _ScalarSets{Float64}()
+    MOI.Utilities.add_set(sets, 1)
+    MOI.Utilities.add_set(sets, 1)
+    MOI.Utilities.add_set(sets, 2)
+    @test MOI.get(sets, MOI.NumberOfConstraints{MOI.SingleVariable,MOI.ZeroOne}()) == 0
+    for (x, S) in zip([2, 1, 0, 0], MOI.Utilities.set_types(sets))
+        @test MOI.get(
+            sets,
+            MOI.NumberOfConstraints{MOI.ScalarAffineFunction{Float64},S}(),
+        ) == x
+    end
+    return
+end
+
+function test_scalar_ListOfConstraintIndices()
+    sets = _ScalarSets{Float64}()
+    MOI.Utilities.add_set(sets, 2)
+    MOI.Utilities.add_set(sets, 4)
+    MOI.Utilities.add_set(sets, 1)
+    for (x, S) in zip([[3], [1], [], [2]], MOI.Utilities.set_types(sets))
+        ci = MOI.get(
+            sets,
+            MOI.ListOfConstraintIndices{MOI.ScalarAffineFunction{Float64},S}(),
+        )
+        @test ci == MOI.ConstraintIndex{MOI.ScalarAffineFunction{Float64},S}.(x)
+    end
+    return
+end
+
+MOI.Utilities.@product_of_sets(
+    _VectorSets,
+    MOI.Nonpositives,
+    MOI.Nonnegatives,
+)
+
+function test_vector_set_index()
+    sets = _VectorSets{Float64}()
+    @test MOI.Utilities.set_index(sets, MOI.EqualTo{Float64}) === nothing
+    @test MOI.Utilities.set_index(sets, MOI.Nonpositives) == 1
+    @test MOI.Utilities.set_index(sets, MOI.Nonnegatives) == 2
+end
+
+function test_vector_set_types()
+    sets = _VectorSets{Float64}()
+    @test MOI.Utilities.set_types(sets) == [
+        MOI.Nonpositives,
+        MOI.Nonnegatives,
+    ]
+end
+
+function test_vector_basic()
+    sets = _VectorSets{Float64}()
+    ci = MOI.ConstraintIndex{
+        MOI.VectorAffineFunction{Float64},
+        MOI.Nonnegatives,
+    }(12345)
+    @test !MOI.is_valid(sets, ci)
+    i = MOI.Utilities.set_index(sets, MOI.Nonnegatives)
+    ci_value = MOI.Utilities.add_set(sets, i, 2)
+    @test ci_value == 0
+    ci = MOI.ConstraintIndex{
+        MOI.VectorAffineFunction{Float64},
+        MOI.Nonnegatives,
+    }(ci_value)
+    @test MOI.is_valid(sets, ci)
+    @test MOI.Utilities.indices(sets, ci) == 1:2
+end
+
+function test_vector_dimension()
+    sets = _VectorSets{Float64}()
+    @test MOI.dimension(sets) == 0
+    MOI.Utilities.add_set(sets, 1, 1)
+    @test MOI.dimension(sets) == 1
+    MOI.Utilities.add_set(sets, 1, 2)
+    @test_broken MOI.dimension(sets) == 3
+    MOI.Utilities.add_set(sets, 2, 3)
+    @test_broken MOI.dimension(sets) == 6
+end
+
+function test_vector_empty()
+    sets = _VectorSets{Float64}()
+    @test MOI.is_empty(sets)
+    MOI.Utilities.add_set(sets, 1, 1)
+    @test !MOI.is_empty(sets)
+    MOI.empty!(sets)
+    @test MOI.is_empty(sets)
+end
+
+function test_vector_ConstraintTypesPresent()
+    sets = _VectorSets{Float64}()
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
+    MOI.Utilities.add_set(sets, 1, 1)
+    @test_broken MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.VectorAffineFunction{Float64},MOI.Nonpositives)]
+end
+
+function test_vector_NumberOfConstraints()
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 1, 2)
+    MOI.Utilities.add_set(sets, 1, 2)
+    MOI.Utilities.add_set(sets, 2, 2)
+    @test MOI.get(sets, MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},MOI.Zeros}()) == 0
+    for (x, S) in zip([2, 1], MOI.Utilities.set_types(sets))
+        @test_broken true == false
+        # @test MOI.get(
+        #     sets,
+        #     MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},S}(),
+        # ) == x
+    end
+    return
+end
+
+function test_vector_ListOfConstraintIndices()
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 2, 2)
+    MOI.Utilities.add_set(sets, 1, 4)
+    MOI.Utilities.add_set(sets, 2, 3)
+    for (x, S) in zip([[2], [0, 6]], MOI.Utilities.set_types(sets))
+        ci = MOI.get(
+            sets,
+            MOI.ListOfConstraintIndices{MOI.VectorAffineFunction{Float64},S}(),
+        )
+        @test_broken ci == MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},S}.(x)
+    end
+    return
+end
+
+end
+
+TestProductOfSets.runtests()

From 3b7a7771e6101a40e71c62797dcc488e25fc25a1 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Mon, 24 May 2021 11:30:57 +1200
Subject: [PATCH 02/11] Fix bugs in .num_rows

---
 src/Utilities/product_of_sets.jl  | 54 +++++++++++++++++++------------
 test/Utilities/product_of_sets.jl | 19 ++++++-----
 2 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 5d8b0bc456..825f904159 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -151,11 +151,23 @@ macro product_of_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
     type_def = :(
-        struct $esc_name{$T} <: $MOIU.OrderedProductOfSets{$T}
+        struct $(esc_name){$(T)} <: $MOIU.OrderedProductOfSets{$(T)}
+            """
+            The number of rows that each set takes.
+            """
             num_rows::Vector{Int}
+
+            """
+            A dictionary which maps the `set_index` and `offset` of a set to the
+            dimension, i.e., `dimension[(set_index,offset)] → dim`.
+            """
             dimension::Dict{Tuple{Int,Int},Int}
-            function $esc_name{$T}() where {$T}
-                return new(zeros(Int, $(1 + length(set_types))), Dict{Int,Int}())
+
+            function $(esc_name){$(T)}() where {$(T)}
+                return new(
+                    zeros(Int, $(length(set_types))),
+                    Dict{Tuple{Int,Int},Int}(),
+                )
             end
         end
     )
@@ -170,19 +182,18 @@ function MOI.empty!(sets::OrderedProductOfSets)
     return
 end
 
-function MOI.dimension(sets::OrderedProductOfSets)
-    for i in 3:length(sets.num_rows)
-        sets.num_rows[i] += sets.num_rows[i-1]
-    end
-    return sets.num_rows[end]
-end
+MOI.dimension(sets::OrderedProductOfSets) = sum(sets.num_rows)
 
 function indices(
     sets::OrderedProductOfSets{T},
     ci::MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},S},
 ) where {T,S}
     i = set_index(sets, S)
-    return sets.num_rows[i] + ci.value + 1
+    offset = 0
+    for j = 1:i
+        offset += sets.num_rows[j]
+    end
+    return offset + ci.value + 1
 end
 
 function indices(
@@ -190,25 +201,28 @@ function indices(
     ci::MOI.ConstraintIndex{MOI.VectorAffineFunction{T},S},
 ) where {T,S}
     i = set_index(sets, S)
-    return (sets.num_rows[i] + ci.value) .+ (1:sets.dimension[(i, ci.value)])
+    offset = ci.value + 1
+    for j = 1:(i-1)
+        offset += sets.num_rows[j]
+    end
+    return offset:(offset + sets.dimension[(i, ci.value)] - 1)
 end
 
 function add_set(sets::OrderedProductOfSets, i)
-    offset = sets.num_rows[i+1]
-    sets.num_rows[i+1] = offset + 1
-    return offset
+    sets.num_rows[i] += 1
+    return sets.num_rows[i] - 1
 end
 
 function add_set(sets::OrderedProductOfSets, i, dim)
-    offset = sets.num_rows[i+1]
-    sets.num_rows[i+1] = offset + dim
-    sets.dimension[(i, offset)] = dim
-    return offset
+    ci = sets.num_rows[i]
+    sets.dimension[(i, ci)] = dim
+    sets.num_rows[i] += dim
+    return ci
 end
 
 function _num_indices(sets::OrderedProductOfSets, ::Type{S}) where {S}
     i = set_index(sets, S)
-    return sets.num_rows[i+1] - sets.num_rows[i]
+    return sets.num_rows[i]
 end
 
 function MOI.get(
@@ -275,7 +289,7 @@ function _range(
             sets,
             i,
             0,
-            sets.num_rows[i+1] - sets.num_rows[i],
+            sets.num_rows[i],
             F,
         )
     end
diff --git a/test/Utilities/product_of_sets.jl b/test/Utilities/product_of_sets.jl
index db4e3983b5..193c70e460 100644
--- a/test/Utilities/product_of_sets.jl
+++ b/test/Utilities/product_of_sets.jl
@@ -162,9 +162,9 @@ function test_vector_dimension()
     MOI.Utilities.add_set(sets, 1, 1)
     @test MOI.dimension(sets) == 1
     MOI.Utilities.add_set(sets, 1, 2)
-    @test_broken MOI.dimension(sets) == 3
+    @test MOI.dimension(sets) == 3
     MOI.Utilities.add_set(sets, 2, 3)
-    @test_broken MOI.dimension(sets) == 6
+    @test MOI.dimension(sets) == 6
 end
 
 function test_vector_empty()
@@ -180,7 +180,7 @@ function test_vector_ConstraintTypesPresent()
     sets = _VectorSets{Float64}()
     @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
     MOI.Utilities.add_set(sets, 1, 1)
-    @test_broken MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.VectorAffineFunction{Float64},MOI.Nonpositives)]
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.VectorAffineFunction{Float64},MOI.Nonpositives)]
 end
 
 function test_vector_NumberOfConstraints()
@@ -190,11 +190,10 @@ function test_vector_NumberOfConstraints()
     MOI.Utilities.add_set(sets, 2, 2)
     @test MOI.get(sets, MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},MOI.Zeros}()) == 0
     for (x, S) in zip([2, 1], MOI.Utilities.set_types(sets))
-        @test_broken true == false
-        # @test MOI.get(
-        #     sets,
-        #     MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},S}(),
-        # ) == x
+        @test MOI.get(
+            sets,
+            MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},S}(),
+        ) == x
     end
     return
 end
@@ -204,12 +203,12 @@ function test_vector_ListOfConstraintIndices()
     MOI.Utilities.add_set(sets, 2, 2)
     MOI.Utilities.add_set(sets, 1, 4)
     MOI.Utilities.add_set(sets, 2, 3)
-    for (x, S) in zip([[2], [0, 6]], MOI.Utilities.set_types(sets))
+    for (x, S) in zip([[0], [0, 2]], MOI.Utilities.set_types(sets))
         ci = MOI.get(
             sets,
             MOI.ListOfConstraintIndices{MOI.VectorAffineFunction{Float64},S}(),
         )
-        @test_broken ci == MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},S}.(x)
+        @test ci == MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},S}.(x)
     end
     return
 end

From d46adf4eb8d220710eb6d6af8c3026e05328679f Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Mon, 24 May 2021 12:04:47 +1200
Subject: [PATCH 03/11] Various updates

---
 docs/src/submodules/Utilities/reference.md |  3 +-
 src/Utilities/product_of_sets.jl           | 87 ++++++++++++++++------
 test/Utilities/product_of_sets.jl          | 84 +++++++++++++++------
 3 files changed, 129 insertions(+), 45 deletions(-)

diff --git a/docs/src/submodules/Utilities/reference.md b/docs/src/submodules/Utilities/reference.md
index a70a845cd5..1d77844202 100644
--- a/docs/src/submodules/Utilities/reference.md
+++ b/docs/src/submodules/Utilities/reference.md
@@ -116,8 +116,9 @@ Utilities.set_types
 Utilities.add_set
 Utilities.indices
 Utilities.MixOfScalarSets
+Utilities.mix_of_scalar_sets
 Utilities.OrderedProductOfSets
-Utilities.OrderedProductOfScalarSets
+Utilities.product_of_sets
 ```
 
 ## Fallbacks
diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 825f904159..318a0990e3 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -48,21 +48,22 @@ function _sets_code(esc_name, T, type_def, set_types...)
     code = Expr(:block, type_def)
     esc_types = esc.(set_types)
     for (i, esc_type) in enumerate(esc_types)
-        method = push!(
+        push!(
             code.args,
             :(
                 function $MOIU.set_index(
-                    ::$esc_name{$T},
-                    ::Type{$esc_type},
+                    ::$esc_name{$(T)},
+                    ::Type{$(esc_type)},
                 ) where {$T}
                     return $i
                 end
             ),
         )
     end
-    push!(code.args, :(function $MOIU.set_types(::$esc_name{$T}) where {$T}
-        return [$(esc_types...)]
-    end))
+    push!(
+        code.args,
+        :($MOIU.set_types(::$esc_name{$T}) where {$T} = [$(esc_types...)]),
+    )
     return code
 end
 
@@ -71,17 +72,41 @@ end
 
 Product of scalar sets in the order the constraints are added, mixing the
 constraints of different types.
+
+Use [`@mix_of_scalar_sets`](@ref) to generate a new subtype.
+
+If the sets are all scalar, use this type instead of [`OrderedProductOfSets`](@ref)
+because it is a more efficient implementation.
 """
 abstract type MixOfScalarSets{T} <: ProductOfSets{T} end
 
+"""
+    @mix_of_scalar_sets(name, set_types...)
+
+Generate a new [`MixOfScalarSets`](@ref) subtype.
+
+## Example
+
+```julia
+@mix_of_scalar_sets(
+    MixedIntegerLinearProgramSets,
+    MOI.GreaterThan{T},
+    MOI.LessThan{T},
+    MOI.EqualTo{T},
+    MOI.Integer,
+)
+```
+"""
 macro mix_of_scalar_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
-    type_def = :(struct $esc_name{$T} <: $MOIU.MixOfScalarSets{$T}
+    type_def = :(struct $(esc_name){$(T)} <: $(MOIU).MixOfScalarSets{$(T)}
+        """
+        `set_ids[i]` maps the row `i` to the corresponding set type.
+        """
         set_ids::Vector{Int}
-        function $esc_name{$T}() where {$T}
-            return new(Int[])
-        end
+
+        $(esc_name){$(T)}() where {$(T)} = new(Int[])
     end)
     return _sets_code(esc_name, T, type_def, set_types...)
 end
@@ -124,8 +149,8 @@ function MOI.get(
 ) where {F,S}
     i = set_index(sets, S)
     return MOI.ConstraintIndex{F,S}[
-        MOI.ConstraintIndex{F,S}(j)
-        for j in eachindex(sets.set_ids) if sets.set_ids[j] == i
+        MOI.ConstraintIndex{F,S}(j) for
+        j in eachindex(sets.set_ids) if sets.set_ids[j] == i
     ]
 end
 
@@ -144,14 +169,36 @@ end
 
 Product of sets in the order the constraints are added, grouping the
 constraints of the same types contiguously.
+
+Use [`@product_of_sets`](@ref) to generate new subtypes.
+
+If the sets are all scalar, use [`MixOfScalarSets`](@ref) because it is a more
+efficient implementation.
 """
 abstract type OrderedProductOfSets{T} <: ProductOfSets{T} end
 
+"""
+    @product_of_sets(name, set_types...)
+
+Generate a new [`OrderedProductOfSets`](@ref) subtype.
+
+## Example
+
+```julia
+@product_of_sets(
+    LinearOrthants,
+    MOI.Zeros,
+    MOI.Nonnegatives,
+    MOI.Nonpositives,
+    MOI.ZeroOne,
+)
+```
+"""
 macro product_of_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
     type_def = :(
-        struct $(esc_name){$(T)} <: $MOIU.OrderedProductOfSets{$(T)}
+        struct $(esc_name){$(T)} <: $(MOIU).OrderedProductOfSets{$(T)}
             """
             The number of rows that each set takes.
             """
@@ -190,7 +237,7 @@ function indices(
 ) where {T,S}
     i = set_index(sets, S)
     offset = 0
-    for j = 1:i
+    for j in 1:(i-1)
         offset += sets.num_rows[j]
     end
     return offset + ci.value + 1
@@ -202,10 +249,10 @@ function indices(
 ) where {T,S}
     i = set_index(sets, S)
     offset = ci.value + 1
-    for j = 1:(i-1)
+    for j in 1:(i-1)
         offset += sets.num_rows[j]
     end
-    return offset:(offset + sets.dimension[(i, ci.value)] - 1)
+    return offset:(offset+sets.dimension[(i, ci.value)]-1)
 end
 
 function add_set(sets::OrderedProductOfSets, i)
@@ -285,13 +332,7 @@ function _range(
     if F != _affine_function_type(T, S) || i === nothing
         return nothing
     else
-        return _range_iterator(
-            sets,
-            i,
-            0,
-            sets.num_rows[i],
-            F,
-        )
+        return _range_iterator(sets, i, 0, sets.num_rows[i], F)
     end
 end
 
diff --git a/test/Utilities/product_of_sets.jl b/test/Utilities/product_of_sets.jl
index 193c70e460..0298301cdc 100644
--- a/test/Utilities/product_of_sets.jl
+++ b/test/Utilities/product_of_sets.jl
@@ -48,14 +48,18 @@ function test_scalar_basic()
     ci = MOI.ConstraintIndex{
         MOI.ScalarAffineFunction{Float64},
         MOI.EqualTo{Float64},
-    }(12345)
+    }(
+        12345,
+    )
     @test !MOI.is_valid(sets, ci)
     i = MOI.Utilities.set_index(sets, MOI.EqualTo{Float64})
     ci_value = MOI.Utilities.add_set(sets, i)
     ci = MOI.ConstraintIndex{
         MOI.ScalarAffineFunction{Float64},
         MOI.EqualTo{Float64},
-    }(ci_value)
+    }(
+        ci_value,
+    )
     @test MOI.is_valid(sets, ci)
     @test MOI.Utilities.indices(sets, ci) == ci.value
 end
@@ -84,7 +88,8 @@ function test_scalar_ConstraintTypesPresent()
     sets = _ScalarSets{Float64}()
     @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
     MOI.Utilities.add_set(sets, 1)
-    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.ScalarAffineFunction{Float64},MOI.EqualTo{Float64})]
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) ==
+          [(MOI.ScalarAffineFunction{Float64}, MOI.EqualTo{Float64})]
 end
 
 function test_scalar_NumberOfConstraints()
@@ -92,7 +97,10 @@ function test_scalar_NumberOfConstraints()
     MOI.Utilities.add_set(sets, 1)
     MOI.Utilities.add_set(sets, 1)
     MOI.Utilities.add_set(sets, 2)
-    @test MOI.get(sets, MOI.NumberOfConstraints{MOI.SingleVariable,MOI.ZeroOne}()) == 0
+    @test MOI.get(
+        sets,
+        MOI.NumberOfConstraints{MOI.SingleVariable,MOI.ZeroOne}(),
+    ) == 0
     for (x, S) in zip([2, 1, 0, 0], MOI.Utilities.set_types(sets))
         @test MOI.get(
             sets,
@@ -121,39 +129,51 @@ MOI.Utilities.@product_of_sets(
     _VectorSets,
     MOI.Nonpositives,
     MOI.Nonnegatives,
+    MOI.EqualTo{T},
 )
 
 function test_vector_set_index()
     sets = _VectorSets{Float64}()
-    @test MOI.Utilities.set_index(sets, MOI.EqualTo{Float64}) === nothing
     @test MOI.Utilities.set_index(sets, MOI.Nonpositives) == 1
     @test MOI.Utilities.set_index(sets, MOI.Nonnegatives) == 2
+    @test MOI.Utilities.set_index(sets, MOI.EqualTo{Float64}) == 3
+    @test MOI.Utilities.set_index(sets, MOI.LessThan{Float64}) === nothing
 end
 
 function test_vector_set_types()
     sets = _VectorSets{Float64}()
-    @test MOI.Utilities.set_types(sets) == [
-        MOI.Nonpositives,
-        MOI.Nonnegatives,
-    ]
+    @test MOI.Utilities.set_types(sets) ==
+          [MOI.Nonpositives, MOI.Nonnegatives, MOI.EqualTo{Float64}]
 end
 
 function test_vector_basic()
     sets = _VectorSets{Float64}()
-    ci = MOI.ConstraintIndex{
-        MOI.VectorAffineFunction{Float64},
-        MOI.Nonnegatives,
-    }(12345)
+    ci =
+        MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},MOI.Nonnegatives}(
+            12345,
+        )
     @test !MOI.is_valid(sets, ci)
     i = MOI.Utilities.set_index(sets, MOI.Nonnegatives)
     ci_value = MOI.Utilities.add_set(sets, i, 2)
     @test ci_value == 0
-    ci = MOI.ConstraintIndex{
-        MOI.VectorAffineFunction{Float64},
-        MOI.Nonnegatives,
-    }(ci_value)
+    ci =
+        MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},MOI.Nonnegatives}(
+            ci_value,
+        )
     @test MOI.is_valid(sets, ci)
     @test MOI.Utilities.indices(sets, ci) == 1:2
+
+    i = MOI.Utilities.set_index(sets, MOI.EqualTo{Float64})
+    ci_value = MOI.Utilities.add_set(sets, i)
+    @test ci_value == 0
+    ci = MOI.ConstraintIndex{
+        MOI.ScalarAffineFunction{Float64},
+        MOI.EqualTo{Float64},
+    }(
+        ci_value,
+    )
+    @test MOI.is_valid(sets, ci)
+    @test MOI.Utilities.indices(sets, ci) == 3
 end
 
 function test_vector_dimension()
@@ -165,6 +185,8 @@ function test_vector_dimension()
     @test MOI.dimension(sets) == 3
     MOI.Utilities.add_set(sets, 2, 3)
     @test MOI.dimension(sets) == 6
+    MOI.Utilities.add_set(sets, 3)
+    @test MOI.dimension(sets) == 7
 end
 
 function test_vector_empty()
@@ -180,7 +202,11 @@ function test_vector_ConstraintTypesPresent()
     sets = _VectorSets{Float64}()
     @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
     MOI.Utilities.add_set(sets, 1, 1)
-    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [(MOI.VectorAffineFunction{Float64},MOI.Nonpositives)]
+    MOI.Utilities.add_set(sets, 3)
+    @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [
+        (MOI.VectorAffineFunction{Float64}, MOI.Nonpositives),
+        (MOI.ScalarAffineFunction{Float64}, MOI.EqualTo{Float64}),
+    ]
 end
 
 function test_vector_NumberOfConstraints()
@@ -188,13 +214,24 @@ function test_vector_NumberOfConstraints()
     MOI.Utilities.add_set(sets, 1, 2)
     MOI.Utilities.add_set(sets, 1, 2)
     MOI.Utilities.add_set(sets, 2, 2)
-    @test MOI.get(sets, MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},MOI.Zeros}()) == 0
-    for (x, S) in zip([2, 1], MOI.Utilities.set_types(sets))
+    MOI.Utilities.add_set(sets, 3)
+    @test MOI.get(
+        sets,
+        MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},MOI.Zeros}(),
+    ) == 0
+    for (x, S) in zip([2, 1], MOI.Utilities.set_types(sets)[1:2])
         @test MOI.get(
             sets,
             MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},S}(),
         ) == x
     end
+    @test MOI.get(
+        sets,
+        MOI.NumberOfConstraints{
+            MOI.ScalarAffineFunction{Float64},
+            MOI.EqualTo{Float64},
+        }(),
+    ) == 1
     return
 end
 
@@ -203,13 +240,18 @@ function test_vector_ListOfConstraintIndices()
     MOI.Utilities.add_set(sets, 2, 2)
     MOI.Utilities.add_set(sets, 1, 4)
     MOI.Utilities.add_set(sets, 2, 3)
-    for (x, S) in zip([[0], [0, 2]], MOI.Utilities.set_types(sets))
+    MOI.Utilities.add_set(sets, 3)
+    for (x, S) in zip([[0], [0, 2]], MOI.Utilities.set_types(sets)[1:2])
         ci = MOI.get(
             sets,
             MOI.ListOfConstraintIndices{MOI.VectorAffineFunction{Float64},S}(),
         )
         @test ci == MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},S}.(x)
     end
+    F = MOI.ScalarAffineFunction{Float64}
+    S = MOI.EqualTo{Float64}
+    @test MOI.get(sets, MOI.ListOfConstraintIndices{F,S}()) ==
+          [MOI.ConstraintIndex{F,S}(0)]
     return
 end
 

From 0d18d1c4f872a12123358c335b8fd8b3a3ccd6db Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Mon, 24 May 2021 12:29:00 +1200
Subject: [PATCH 04/11] fix docs

---
 docs/src/submodules/Utilities/reference.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/src/submodules/Utilities/reference.md b/docs/src/submodules/Utilities/reference.md
index 1d77844202..9bc450b1be 100644
--- a/docs/src/submodules/Utilities/reference.md
+++ b/docs/src/submodules/Utilities/reference.md
@@ -116,9 +116,9 @@ Utilities.set_types
 Utilities.add_set
 Utilities.indices
 Utilities.MixOfScalarSets
-Utilities.mix_of_scalar_sets
+Utilities.@mix_of_scalar_sets
 Utilities.OrderedProductOfSets
-Utilities.product_of_sets
+Utilities.@product_of_sets
 ```
 
 ## Fallbacks

From b972f390a5252870c797af29065c7205b433d083 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Mon, 24 May 2021 14:10:07 +1200
Subject: [PATCH 05/11] Fix tests

---
 test/Utilities/matrix_of_constraints.jl | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/test/Utilities/matrix_of_constraints.jl b/test/Utilities/matrix_of_constraints.jl
index d235c8fec9..ba53705ce2 100644
--- a/test/Utilities/matrix_of_constraints.jl
+++ b/test/Utilities/matrix_of_constraints.jl
@@ -159,13 +159,7 @@ MOIU.@mix_of_scalar_sets(
     MOI.LessThan{T},
     MOI.Interval{T},
 )
-MOIU.@product_of_scalar_sets(
-    OrdsLP,
-    MOI.EqualTo{T},
-    MOI.GreaterThan{T},
-    MOI.LessThan{T},
-    MOI.Interval{T},
-)
+
 MOIU.@product_of_sets(
     OrdLP,
     MOI.EqualTo{T},
@@ -187,8 +181,7 @@ MOIU.@product_of_sets(
     )
     blp = MOI.Utilities.Box([5, 0, -Inf, 6], [5, Inf, 0, 7])
     F = MOI.ScalarAffineFunction{Float64}
-    @testset "$SetType" for SetType in
-                            [MixLP{Float64}, OrdsLP{Float64}, OrdLP{Float64}]
+    @testset "$SetType" for SetType in [MixLP{Float64}, OrdLP{Float64}]
         _test(
             MOIT.linear2test,
             MOI.Utilities.Box{Float64},

From a799cf1cd22a6f97430e56ebb083658042ef0b1f Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Tue, 25 May 2021 09:12:14 +1200
Subject: [PATCH 06/11] Use ordered num_rows

---
 src/Utilities/matrix_of_constraints.jl |   7 ++
 src/Utilities/product_of_sets.jl       | 115 +++++++++++++------------
 test/Utilities/product_of_sets.jl      |  70 +++++++++------
 3 files changed, 113 insertions(+), 79 deletions(-)

diff --git a/src/Utilities/matrix_of_constraints.jl b/src/Utilities/matrix_of_constraints.jl
index ed5f014754..4da4e72fe3 100644
--- a/src/Utilities/matrix_of_constraints.jl
+++ b/src/Utilities/matrix_of_constraints.jl
@@ -74,6 +74,7 @@ The `.sets::ST` type must implement:
  * [`MOI.Utilities.set_index`](@ref)
  * [`MOI.Utilities.add_set`](@ref)
  * [`MOI.Utilities.rows`](@ref)
+ * [`MOI.Utilities.final_touch`](@ref)
 """
 mutable struct MatrixOfConstraints{T,AT,BT,ST} <: MOI.ModelLike
     coefficients::AT
@@ -135,6 +136,11 @@ function load_terms end
     final_touch(coefficients)::Nothing
 
 Informs the `coefficients` that all functions have been added with `load_terms`.
+No more modification is allowed unless `MOI.empty!` is called.
+
+    final_touch(sets)::Nothing
+
+Informs the `sets` that all functions have been added with `add_set`.
 No more modification is allowed unless `MOI.empty!` is called.
 """
 function final_touch end
@@ -392,6 +398,7 @@ function pass_nonvariable_constraints(
 end
 
 function final_touch(model::MatrixOfConstraints, index_map)
+    final_touch(model.sets)
     num_rows = MOI.dimension(model.sets)
     resize!(model.constants, num_rows)
     set_number_of_rows(model.coefficients, num_rows)
diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 4527f4fe9c..5736255f3f 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -87,6 +87,8 @@ function add_set(sets::MixOfScalarSets, i)
     return length(sets.set_ids)
 end
 
+final_touch(::MixOfScalarSets) = nothing
+
 function MOI.get(
     sets::MixOfScalarSets{T},
     ::MOI.ListOfConstraintTypesPresent,
@@ -112,8 +114,8 @@ function MOI.get(
 ) where {F,S}
     i = set_index(sets, S)
     return MOI.ConstraintIndex{F,S}[
-        MOI.ConstraintIndex{F,S}(j) for
-        j in eachindex(sets.set_ids) if sets.set_ids[j] == i
+        MOI.ConstraintIndex{F,S}(ci) for
+        (ci, set_type) in enumerate(sets.set_ids) if set_type == i
     ]
 end
 
@@ -122,9 +124,10 @@ function MOI.is_valid(
     ci::MOI.ConstraintIndex{F,S},
 ) where {F,S}
     i = set_index(sets, S)
-    return i !== nothing &&
-           ci.value in eachindex(sets.set_ids) &&
-           sets.set_ids[ci.value] == i
+    if i === nothing
+        return false
+    end
+    return ci.value in eachindex(sets.set_ids) && sets.set_ids[ci.value] == i
 end
 
 """
@@ -161,9 +164,11 @@ macro product_of_sets(name, set_types...)
     esc_name = esc(name)
     T = esc(:T)
     type_def = :(
-        struct $(esc_name){$(T)} <: $(MOIU).OrderedProductOfSets{$(T)}
+        mutable struct $(esc_name){$(T)} <: $(MOIU).OrderedProductOfSets{$(T)}
             """
-            The number of rows that each set takes.
+            During the copy, this counts the number of rows corresponding to
+            each set. At the end of copy, `final_touch` is called, which
+            converts this list into a cumulative ordering.
             """
             num_rows::Vector{Int}
 
@@ -173,10 +178,16 @@ macro product_of_sets(name, set_types...)
             """
             dimension::Dict{Tuple{Int,Int},Int}
 
+            """
+            A sanity bit to check that we don't call functions out-of-order.
+            """
+            final_touch::Bool
+
             function $(esc_name){$(T)}() where {$(T)}
                 return new(
                     zeros(Int, $(length(set_types))),
                     Dict{Tuple{Int,Int},Int}(),
+                    false,
                 )
             end
         end
@@ -189,59 +200,74 @@ MOI.is_empty(sets::OrderedProductOfSets) = all(iszero, sets.num_rows)
 function MOI.empty!(sets::OrderedProductOfSets)
     fill!(sets.num_rows, 0)
     empty!(sets.dimension)
+    sets.final_touch = false
     return
 end
 
-MOI.dimension(sets::OrderedProductOfSets) = sum(sets.num_rows)
+function MOI.dimension(sets::OrderedProductOfSets)
+    @assert sets.final_touch
+    return sets.num_rows[end]
+end
 
 function rows(
     sets::OrderedProductOfSets{T},
     ci::MOI.ConstraintIndex{MOI.ScalarAffineFunction{T},S},
 ) where {T,S}
+    @assert sets.final_touch
     i = set_index(sets, S)
-    offset = 0
-    for j in 1:(i-1)
-        offset += sets.num_rows[j]
-    end
-    return offset + ci.value + 1
+    return (i == 1 ? 0 : sets.num_rows[i-1]) + ci.value + 1
 end
 
 function rows(
     sets::OrderedProductOfSets{T},
     ci::MOI.ConstraintIndex{MOI.VectorAffineFunction{T},S},
 ) where {T,S}
+    @assert sets.final_touch
     i = set_index(sets, S)
-    offset = ci.value + 1
-    for j in 1:(i-1)
-        offset += sets.num_rows[j]
-    end
-    return offset:(offset+sets.dimension[(i, ci.value)]-1)
+    offset = i == 1 ? 0 : sets.num_rows[i-1]
+    return (ci.value + offset) .+ 1:sets.dimension[(i, ci.value)]
 end
 
 function add_set(sets::OrderedProductOfSets, i)
+    @assert !sets.final_touch
     sets.num_rows[i] += 1
     return sets.num_rows[i] - 1
 end
 
 function add_set(sets::OrderedProductOfSets, i, dim)
+    @assert !sets.final_touch
     ci = sets.num_rows[i]
     sets.dimension[(i, ci)] = dim
     sets.num_rows[i] += dim
     return ci
 end
 
+function final_touch(sets::OrderedProductOfSets)
+    @assert !sets.final_touch
+    for i in 2:length(sets.num_rows)
+        sets.num_rows[i] += sets.num_rows[i-1]
+    end
+    sets.final_touch = true
+    return
+end
+
 function _num_rows(sets::OrderedProductOfSets, ::Type{S}) where {S}
+    @assert sets.final_touch
     i = set_index(sets, S)
-    return sets.num_rows[i]
+    if i == 1
+        return sets.num_rows[1]
+    end
+    return sets.num_rows[i] - sets.num_rows[i-1]
 end
 
 function MOI.get(
     sets::OrderedProductOfSets{T},
     ::MOI.ListOfConstraintTypesPresent,
 ) where {T}
+    @assert sets.final_touch
     return Tuple{DataType,DataType}[
         (_affine_function_type(T, S), S) for
-        S in set_types(sets) if !iszero(_num_rows(sets, S))
+        S in set_types(sets) if _num_rows(sets, S) > 0
     ]
 end
 
@@ -257,13 +283,12 @@ Base.IteratorSize(::_UnevenIterator) = Base.SizeUnknown()
 function Base.iterate(it::_UnevenIterator, cur = it.start)
     if cur >= it.stop
         return nothing
-    else
-        return (cur, cur + it.dimension[(it.i, cur)])
     end
+    return (cur, cur + it.dimension[(it.i, cur)])
 end
 
-function Base.in(x, it::_UnevenIterator)
-    return x in it.start:(it.stop-1) && haskey(it.dimension, (it.i, x))
+function Base.in(x::Int64, it::_UnevenIterator)
+    return it.start <= x < it.stop && haskey(it.dimension, (it.i, x))
 end
 
 function _range_iterator(
@@ -286,19 +311,19 @@ function _range_iterator(
     return _UnevenIterator(i, start, stop, sets.dimension)
 end
 
-function _range(
+function _range_iterator(
     sets::OrderedProductOfSets{T},
     ::Type{F},
     ::Type{S},
 ) where {T,F,S}
     i = set_index(sets, S)
-    if F != _affine_function_type(T, S) || i === nothing
-        return nothing
-    else
-        return _range_iterator(sets, i, 0, sets.num_rows[i], F)
+    if i === nothing || F != _affine_function_type(T, S)
+        return
     end
+    return _range_iterator(sets, i, 0, _num_rows(sets, S), F)
 end
 
+_length(::Nothing) = 0
 _length(r::UnitRange) = length(r)
 _length(r::_UnevenIterator) = count(_ -> true, r)
 
@@ -306,36 +331,19 @@ function MOI.get(
     sets::OrderedProductOfSets,
     ::MOI.NumberOfConstraints{F,S},
 ) where {F,S}
-    r = _range(sets, F, S)
-    if r === nothing
-        return 0
-    else
-        return _length(r)
-    end
-end
-
-function _empty(
-    ::OrderedProductOfSets{T},
-    ::Type{<:MOI.ScalarAffineFunction},
-) where {T}
-    return 1:0
-end
-
-function _empty(
-    sets::OrderedProductOfSets{T},
-    ::Type{<:MOI.VectorAffineFunction},
-) where {T}
-    return _UnevenIterator(1, 1, 0, sets.dimension)
+    @assert sets.final_touch
+    r = _range_iterator(sets, F, S)
+    return _length(r)
 end
 
 function MOI.get(
     sets::OrderedProductOfSets,
     ::MOI.ListOfConstraintIndices{F,S},
 ) where {F,S}
-    rows = _range(sets, F, S)
+    @assert sets.final_touch
+    rows = _range_iterator(sets, F, S)
     if rows === nothing
-        # Empty iterator
-        rows = _empty(sets, F)
+        return MOI.ConstraintIndex{F,S}[]
     end
     return MOI.ConstraintIndex{F,S}.(rows)
 end
@@ -344,6 +352,7 @@ function MOI.is_valid(
     sets::OrderedProductOfSets,
     ci::MOI.ConstraintIndex{F,S},
 ) where {F,S}
-    r = _range(sets, F, S)
+    @assert sets.final_touch
+    r = _range_iterator(sets, F, S)
     return r !== nothing && ci.value in r
 end
diff --git a/test/Utilities/product_of_sets.jl b/test/Utilities/product_of_sets.jl
index 0298301cdc..ca1f1bde77 100644
--- a/test/Utilities/product_of_sets.jl
+++ b/test/Utilities/product_of_sets.jl
@@ -61,7 +61,7 @@ function test_scalar_basic()
         ci_value,
     )
     @test MOI.is_valid(sets, ci)
-    @test MOI.Utilities.indices(sets, ci) == ci.value
+    @test MOI.Utilities.rows(sets, ci) == ci.value
 end
 
 function test_scalar_dimension()
@@ -97,6 +97,7 @@ function test_scalar_NumberOfConstraints()
     MOI.Utilities.add_set(sets, 1)
     MOI.Utilities.add_set(sets, 1)
     MOI.Utilities.add_set(sets, 2)
+    MOI.Utilities.final_touch(sets)
     @test MOI.get(
         sets,
         MOI.NumberOfConstraints{MOI.SingleVariable,MOI.ZeroOne}(),
@@ -148,44 +149,56 @@ end
 
 function test_vector_basic()
     sets = _VectorSets{Float64}()
-    ci =
-        MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},MOI.Nonnegatives}(
-            12345,
-        )
-    @test !MOI.is_valid(sets, ci)
-    i = MOI.Utilities.set_index(sets, MOI.Nonnegatives)
-    ci_value = MOI.Utilities.add_set(sets, i, 2)
-    @test ci_value == 0
-    ci =
-        MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},MOI.Nonnegatives}(
-            ci_value,
-        )
-    @test MOI.is_valid(sets, ci)
-    @test MOI.Utilities.indices(sets, ci) == 1:2
-
-    i = MOI.Utilities.set_index(sets, MOI.EqualTo{Float64})
-    ci_value = MOI.Utilities.add_set(sets, i)
-    @test ci_value == 0
-    ci = MOI.ConstraintIndex{
-        MOI.ScalarAffineFunction{Float64},
-        MOI.EqualTo{Float64},
-    }(
-        ci_value,
+    nonneg_i = MOI.Utilities.add_set(
+        sets,
+        MOI.Utilities.set_index(sets, MOI.Nonnegatives),
+        2,
     )
-    @test MOI.is_valid(sets, ci)
-    @test MOI.Utilities.indices(sets, ci) == 3
+    equalto_i = MOI.Utilities.add_set(
+        sets,
+        MOI.Utilities.set_index(sets, MOI.EqualTo{Float64}),
+    )
+    MOI.Utilities.final_touch(sets)
+    VAF = MOI.VectorAffineFunction{Float64}
+    @test !MOI.is_valid(sets, MOI.ConstraintIndex{VAF,MOI.Nonnegatives}(12345))
+    nonneg_ci = MOI.ConstraintIndex{VAF,MOI.Nonnegatives}(nonneg_i)
+    @test MOI.is_valid(sets, nonneg_ci)
+    @test MOI.Utilities.rows(sets, nonneg_ci) == 1:2
+    SAF = MOI.ScalarAffineFunction{Float64}
+    equalto_ci = MOI.ConstraintIndex{SAF,MOI.EqualTo{Float64}}(equalto_i)
+    @test MOI.is_valid(sets, equalto_ci)
+    @test MOI.Utilities.rows(sets, equalto_ci) == 3
 end
 
 function test_vector_dimension()
     sets = _VectorSets{Float64}()
+    MOI.Utilities.final_touch(sets)
     @test MOI.dimension(sets) == 0
+
+    sets = _VectorSets{Float64}()
     MOI.Utilities.add_set(sets, 1, 1)
+    MOI.Utilities.final_touch(sets)
     @test MOI.dimension(sets) == 1
+
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 1, 1)
     MOI.Utilities.add_set(sets, 1, 2)
+    MOI.Utilities.final_touch(sets)
     @test MOI.dimension(sets) == 3
+
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 1, 1)
+    MOI.Utilities.add_set(sets, 1, 2)
     MOI.Utilities.add_set(sets, 2, 3)
+    MOI.Utilities.final_touch(sets)
     @test MOI.dimension(sets) == 6
+
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 1, 1)
+    MOI.Utilities.add_set(sets, 1, 2)
+    MOI.Utilities.add_set(sets, 2, 3)
     MOI.Utilities.add_set(sets, 3)
+    MOI.Utilities.final_touch(sets)
     @test MOI.dimension(sets) == 7
 end
 
@@ -200,9 +213,12 @@ end
 
 function test_vector_ConstraintTypesPresent()
     sets = _VectorSets{Float64}()
+    MOI.Utilities.final_touch(sets)
     @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == []
+    sets = _VectorSets{Float64}()
     MOI.Utilities.add_set(sets, 1, 1)
     MOI.Utilities.add_set(sets, 3)
+    MOI.Utilities.final_touch(sets)
     @test MOI.get(sets, MOI.ListOfConstraintTypesPresent()) == [
         (MOI.VectorAffineFunction{Float64}, MOI.Nonpositives),
         (MOI.ScalarAffineFunction{Float64}, MOI.EqualTo{Float64}),
@@ -215,6 +231,7 @@ function test_vector_NumberOfConstraints()
     MOI.Utilities.add_set(sets, 1, 2)
     MOI.Utilities.add_set(sets, 2, 2)
     MOI.Utilities.add_set(sets, 3)
+    MOI.Utilities.final_touch(sets)
     @test MOI.get(
         sets,
         MOI.NumberOfConstraints{MOI.VectorAffineFunction{Float64},MOI.Zeros}(),
@@ -241,6 +258,7 @@ function test_vector_ListOfConstraintIndices()
     MOI.Utilities.add_set(sets, 1, 4)
     MOI.Utilities.add_set(sets, 2, 3)
     MOI.Utilities.add_set(sets, 3)
+    MOI.Utilities.final_touch(sets)
     for (x, S) in zip([[0], [0, 2]], MOI.Utilities.set_types(sets)[1:2])
         ci = MOI.get(
             sets,

From 999b9ebba899ffc7c198cd1791ea185ed5f8b2ab Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Tue, 25 May 2021 09:13:33 +1200
Subject: [PATCH 07/11] Fix formatting

---
 src/Utilities/product_of_sets.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 5736255f3f..5725a85894 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -225,7 +225,7 @@ function rows(
     @assert sets.final_touch
     i = set_index(sets, S)
     offset = i == 1 ? 0 : sets.num_rows[i-1]
-    return (ci.value + offset) .+ 1:sets.dimension[(i, ci.value)]
+    return (ci.value+offset).+1:sets.dimension[(i, ci.value)]
 end
 
 function add_set(sets::OrderedProductOfSets, i)

From b8d6c1d0800a0be315ce5f7dec1d1cab7d658e16 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Tue, 25 May 2021 10:01:14 +1200
Subject: [PATCH 08/11] Fix formatting bug

---
 src/Utilities/product_of_sets.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 5725a85894..78a3e19f4c 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -225,7 +225,7 @@ function rows(
     @assert sets.final_touch
     i = set_index(sets, S)
     offset = i == 1 ? 0 : sets.num_rows[i-1]
-    return (ci.value+offset).+1:sets.dimension[(i, ci.value)]
+    return (ci.value+offset).+(1:sets.dimension[(i, ci.value)])
 end
 
 function add_set(sets::OrderedProductOfSets, i)

From b38accdf4fe5a7ff393e894328ec4c2a8117e779 Mon Sep 17 00:00:00 2001
From: odow <o.dowson@gmail.com>
Date: Tue, 25 May 2021 10:10:48 +1200
Subject: [PATCH 09/11] Add more tests

---
 test/Utilities/product_of_sets.jl | 32 ++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/test/Utilities/product_of_sets.jl b/test/Utilities/product_of_sets.jl
index ca1f1bde77..82a288bd51 100644
--- a/test/Utilities/product_of_sets.jl
+++ b/test/Utilities/product_of_sets.jl
@@ -259,20 +259,38 @@ function test_vector_ListOfConstraintIndices()
     MOI.Utilities.add_set(sets, 2, 3)
     MOI.Utilities.add_set(sets, 3)
     MOI.Utilities.final_touch(sets)
+    VAF = MOI.VectorAffineFunction{Float64}
+    @test MOI.get(sets, MOI.ListOfConstraintIndices{VAF,MOI.Zeros}()) ==
+          MOI.ConstraintIndex{VAF,MOI.Zeros}[]
     for (x, S) in zip([[0], [0, 2]], MOI.Utilities.set_types(sets)[1:2])
-        ci = MOI.get(
-            sets,
-            MOI.ListOfConstraintIndices{MOI.VectorAffineFunction{Float64},S}(),
-        )
-        @test ci == MOI.ConstraintIndex{MOI.VectorAffineFunction{Float64},S}.(x)
+        ci = MOI.get(sets, MOI.ListOfConstraintIndices{VAF,S}())
+        @test ci == MOI.ConstraintIndex{VAF,S}.(x)
     end
-    F = MOI.ScalarAffineFunction{Float64}
-    S = MOI.EqualTo{Float64}
+    F, S = MOI.ScalarAffineFunction{Float64}, MOI.EqualTo{Float64}
     @test MOI.get(sets, MOI.ListOfConstraintIndices{F,S}()) ==
           [MOI.ConstraintIndex{F,S}(0)]
     return
 end
 
+"""
+    test_vector_ListOfConstraintIndices2()
+
+Test a more complicated sequence of dimensions to check the `_UnevenIterator`
+works appropriately.
+"""
+function test_vector_ListOfConstraintIndices2()
+    sets = _VectorSets{Float64}()
+    MOI.Utilities.add_set(sets, 2, 2)
+    MOI.Utilities.add_set(sets, 2, 3)
+    MOI.Utilities.add_set(sets, 2, 2)
+    MOI.Utilities.add_set(sets, 2, 4)
+    MOI.Utilities.final_touch(sets)
+    S = MOI.Utilities.set_types(sets)[2]
+    VAF = MOI.VectorAffineFunction{Float64}
+    indices = MOI.get(sets, MOI.ListOfConstraintIndices{VAF,S}())
+    @test indices == MOI.ConstraintIndex{VAF,S}.([0, 2, 5, 7])
+end
+
 end
 
 TestProductOfSets.runtests()

From 04c4ea426a457bad470af30255e60d73cab3060b Mon Sep 17 00:00:00 2001
From: Oscar Dowson <odow@users.noreply.github.com>
Date: Tue, 25 May 2021 13:23:03 +1200
Subject: [PATCH 10/11] Update product_of_sets.jl

---
 src/Utilities/product_of_sets.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 78a3e19f4c..217bfdcc3f 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -225,7 +225,7 @@ function rows(
     @assert sets.final_touch
     i = set_index(sets, S)
     offset = i == 1 ? 0 : sets.num_rows[i-1]
-    return (ci.value+offset).+(1:sets.dimension[(i, ci.value)])
+    return (ci.value + offset) .+ (1:sets.dimension[(i, ci.value)])
 end
 
 function add_set(sets::OrderedProductOfSets, i)

From 80abb2026d37ac1d22d3b27b9f9a75a9f777d774 Mon Sep 17 00:00:00 2001
From: Oscar Dowson <odow@users.noreply.github.com>
Date: Tue, 25 May 2021 13:23:58 +1200
Subject: [PATCH 11/11] Update product_of_sets.jl

---
 src/Utilities/product_of_sets.jl | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/Utilities/product_of_sets.jl b/src/Utilities/product_of_sets.jl
index 217bfdcc3f..ae8bf3a3b1 100644
--- a/src/Utilities/product_of_sets.jl
+++ b/src/Utilities/product_of_sets.jl
@@ -37,9 +37,6 @@ Product of scalar sets in the order the constraints are added, mixing the
 constraints of different types.
 
 Use [`@mix_of_scalar_sets`](@ref) to generate a new subtype.
-
-If the sets are all scalar, use this type instead of [`OrderedProductOfSets`](@ref)
-because it is a more efficient implementation.
 """
 abstract type MixOfScalarSets{T} <: ProductOfSets{T} end
 
@@ -137,9 +134,6 @@ Product of sets in the order the constraints are added, grouping the
 constraints of the same types contiguously.
 
 Use [`@product_of_sets`](@ref) to generate new subtypes.
-
-If the sets are all scalar, use [`MixOfScalarSets`](@ref) because it is a more
-efficient implementation.
 """
 abstract type OrderedProductOfSets{T} <: ProductOfSets{T} end