From 3ef0d90a863411803c3a3747f346218656181462 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Beno=C3=AEt=20Legat?= <benoit.legat@gmail.com>
Date: Fri, 23 Aug 2019 11:33:39 +0200
Subject: [PATCH] Don't change order of variables when NLPBlock is used

---
 src/Utilities/copy.jl  | 61 +++++++++++++++++++++++++-----------------
 test/Utilities/copy.jl | 22 +++++++++++++++
 2 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/src/Utilities/copy.jl b/src/Utilities/copy.jl
index 824084fdba..ed0f09c030 100644
--- a/src/Utilities/copy.jl
+++ b/src/Utilities/copy.jl
@@ -267,6 +267,21 @@ function pass_constraints(
     end
 end
 
+function copy_free_variables(dest::MOI.ModelLike, idxmap::IndexMap, vis_src, copy_variables::Function)
+    if length(vis_src) != length(keys(idxmap.varmap))
+        vars = copy_variables(dest, length(vis_src) - length(idxmap.varmap))
+        i = 1
+        for vi in vis_src
+            if !haskey(idxmap.varmap, vi)
+                idxmap.varmap[vi] = vars[i]
+                i += 1
+            end
+        end
+        @assert i == length(vars) + 1
+        @assert length(vis_src) == length(idxmap.varmap)
+    end
+end
+
 function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike)
     Base.depwarn("default_copy_to(dest, src) is deprecated, use default_copy_to(dest, src, true) instead or default_copy_to(dest, src, false) if you do not want to copy names.", :default_copy_to)
     default_copy_to(dest, src, true)
@@ -292,24 +307,29 @@ function default_copy_to(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bo
     vector_of_variables_types = [S for (F, S) in constraint_types
                                  if F == MOI.VectorOfVariables]
 
-    vector_of_variables_not_added = [
-        copy_vector_of_variables(dest, src, idxmap, S)
-        for S in vector_of_variables_types
-    ]
-    single_variable_not_added = [
-        copy_single_variable(dest, src, idxmap, S)
-        for S in single_variable_types
-    ]
-
-    if length(vis_src) != length(keys(idxmap.varmap))
-        # Copy free variables
-        variables_not_added = setdiff(Set(vis_src), keys(idxmap.varmap))
-        vars = MOI.add_variables(dest, length(variables_not_added))
-        for (vi, var) in zip(variables_not_added, vars)
-            idxmap.varmap[vi] = var
-        end
+    # The `NLPBlock` assumes that the order of variables does not change (#849)
+    if MOI.NLPBlock() in MOI.get(src, MOI.ListOfModelAttributesSet())
+        vector_of_variables_not_added = [
+            MOI.get(src, MOI.ListOfConstraintIndices{MOI.VectorOfVariables, S}())
+            for S in vector_of_variables_types
+        ]
+        single_variable_not_added = [
+            MOI.get(src, MOI.ListOfConstraintIndices{MOI.SingleVariable, S}())
+            for S in single_variable_types
+        ]
+    else
+        vector_of_variables_not_added = [
+            copy_vector_of_variables(dest, src, idxmap, S)
+            for S in vector_of_variables_types
+        ]
+        single_variable_not_added = [
+            copy_single_variable(dest, src, idxmap, S)
+            for S in single_variable_types
+        ]
     end
 
+    copy_free_variables(dest, idxmap, vis_src, MOI.add_variables)
+
     # Copy variable attributes
     pass_attributes(dest, src, copy_names, idxmap, vis_src)
 
@@ -652,14 +672,7 @@ function allocate_load(dest::MOI.ModelLike, src::MOI.ModelLike, copy_names::Bool
             allocate_single_variable(dest, src, idxmap, S)
     end
 
-    if length(vis_src) != length(keys(idxmap.varmap))
-        # Allocate free variables
-        variables_not_added = setdiff(Set(vis_src), keys(idxmap.varmap))
-        vars = allocate_variables(dest, length(variables_not_added))
-        for (vi, var) in zip(variables_not_added, vars)
-            idxmap.varmap[vi] = var
-        end
-    end
+    copy_free_variables(dest, idxmap, vis_src, allocate_variables)
 
     # Allocate variable attributes
     pass_attributes(dest, src, copy_names, idxmap, vis_src, allocate)
diff --git a/test/Utilities/copy.jl b/test/Utilities/copy.jl
index ba68b9ca77..4f9eb97146 100644
--- a/test/Utilities/copy.jl
+++ b/test/Utilities/copy.jl
@@ -57,3 +57,25 @@ end
     MOIT.failcopytestca(mock)
     MOIT.copytest(mock, MOIU.Model{Float64}())
 end
+
+struct DummyEvaluator <: MOI.AbstractNLPEvaluator end
+
+@testset "Create variables in same ordering when NLPBlock is used (#849)" begin
+    model = MOIU.UniversalFallback(MOIU.Model{Float64}())
+    a = MOI.add_variable(model)
+    b, c = MOI.add_variables(model, 2)
+    x, cx = MOI.add_constrained_variable(model, MOI.GreaterThan(0.0))
+    y, cy = MOI.add_constrained_variables(model, MOI.Nonnegatives(1))
+    nlp_data = MOI.NLPBlockData(
+        [MOI.NLPBoundsPair(0.0, 1.0) for i in 1:5],
+        DummyEvaluator(), false)
+    MOI.set(model, MOI.NLPBlock(), nlp_data)
+    copy = MOIU.UniversalFallback(MOIU.Model{Float64}())
+    index_map = MOIU.default_copy_to(copy, model, true)
+    for vi in [a, b, c, x, y[1]]
+        @test index_map[vi] == vi
+    end
+    for ci in [cx, cy]
+        @test index_map[ci] == ci
+    end
+end