[very breaking] Fix dual sign of equalities

docs/src/manual.md

@@ -205,7 +205,7 @@ When define MOI attributes to communicate information in MOI, layers like Dualiz
 these attributes to the right variables or constraints before or after their transformation.
 So the best practice is to define such variable or constraint attributes and attempt to `MOI.get`
 or `MOI.set` them. They is an API that you need to define for these attributes that contains the
-functions are `dual_attribute`, `dual_attribute_value`,
+functions are `dual_attribute`,
 `constrained_variable_dual_attribute`, `fixed_variable_value`, `fixed_constrained_variables_get`,
 `equality_constraint_get`. That's a lot of functions, some of which may never be useful for
 your specific attributes (e.g., if it is never used on an equality constraint) so no need

docs/src/mathematical_background.md

@@ -54,8 +54,14 @@ and the dual is:
 ```
 
 Note that the equality constraints have minus signs that could be flipped for
-simplicity. However, for generality, we keep the minus signs so that the models
-displayed here precisely match the outputs of the package.
+simplicity, because negating both sides of an equality does not change it.
+However, we keep the minus signs, so that the term ``- \sum_{i=1}^m A_i^T y_i``
+appears with the same sign in every dual constraint, whether that constraint
+ends up being an equality (as here) or a membership in a nontrivial cone (as in
+the compact form below). This makes the dual constraints directly readable as
+the stationarity rows of the KKT conditions, which matters when the dual is
+used to build a complementarity system. It also means the models displayed
+here precisely match the outputs of the package.
 
 A linear inequality constraint ``a^T x + b \ge c`` should be interpreted as
 ``a^T x + b - c \in \mathbb{R}_+``, and similarly ``a^T x + b \le c`` should be
@@ -179,7 +185,7 @@ and the dual is:
 \end{align}
 ```
 
-Note that signs changed in the constraints of the dual compared to the standard form. This is because the standard form would have negative signs in all terms in a equality constraint, which were inverted for simplicity. However, in the compact form, this operation is not allowed because it would change a nontrivial cone ``\mathcal{C}_i``.
+Note that the constraints of the dual carry the same signs as in the standard form. Negating both sides of an equality would be allowed in the standard form, but it is not allowed here, because it would change a nontrivial cone ``\mathcal{V}_j^*``. Keeping a single convention for both forms means that adding or removing an explicit constraint on a primal variable does not flip the signs of the corresponding dual constraint.
 
 ##### Linear Programming
 

docs/src/reference.md

@@ -75,8 +75,6 @@ Dualization.PrimalDualMap
 
 ```@docs
 Dualization.dual_attribute
-Dualization.dual_attribute_value_get
-Dualization.dual_attribute_value_set
 Dualization.constrained_variable_dual_attribute
 Dualization.fixed_variable_value
 Dualization.fixed_constrained_variables_get

src/attributes.jl

@@ -1,6 +1,3 @@
-_minus(::Nothing) = nothing
-_minus(x) = -x
-
 function _variable_attribute(attr::MOI.ConstraintPrimal)
     return MOI.VariablePrimal(attr.result_index)
 end
@@ -57,34 +54,6 @@ function dual_attribute(::MOI.ConstraintDualStart)
     return MOI.VariablePrimalStart()
 end
 
-"""
-    dual_attribute_value_set(attr::MOI.AbstractVariableAttribute, value)
-
-Used as pre-processing for `MOI.set`ting `value` for a variable.
-"""
-function dual_attribute_value_set end
-
-"""
-    dual_attribute_value_get(attr::MOI.AbstractVariableAttribute, value)
-
-Used as pre-processing for `MOI.get`ting `value` for a variable.
-"""
-function dual_attribute_value_get end
-
-function dual_attribute_value_set(
-    ::Union{MOI.VariablePrimal,MOI.VariablePrimalStart},
-    value,
-)
-    return _minus(value)
-end
-
-function dual_attribute_value_get(
-    ::Union{MOI.VariablePrimal,MOI.VariablePrimalStart},
-    value,
-)
-    return _minus(value)
-end
-
 """
     constrained_variable_dual_attribute(attr::MOI.AbstractConstraintAttribute)
 
@@ -157,7 +126,7 @@ function MOI.set(
         optimizer.dual_problem.dual_model,
         dual_attribute(attr),
         data.dual_constraint,
-        dual_attribute_value_set(attr, value),
+        value,
     )
     return
 end
@@ -170,14 +139,10 @@ function MOI.get(
     primal_dual_map = optimizer.dual_problem.primal_dual_map
     data = primal_dual_map.primal_variable_data[vi]
     if isnothing(data.primal_constrained_variable_constraint)
-        # Classical free variable
-        return dual_attribute_value_get(
-            attr,
-            MOI.get(
-                optimizer.dual_problem.dual_model,
-                dual_attribute(attr),
-                data.dual_constraint,
-            ),
+        return MOI.get(
+            optimizer.dual_problem.dual_model,
+            dual_attribute(attr),
+            data.dual_constraint,
         )
     end
     if isnothing(data.dual_constraint)

src/dual_equality_constraints.jl

@@ -22,9 +22,14 @@ function _add_dual_equality_constraints(
     non_parameter_variables = setdiff(all_variables, variable_parameters)
 
     # Loop at every constraint to collect the scalar affine terms in the
-    # `scalar_affine_terms` list (a dics mapping variable index to 
+    # `scalar_affine_terms` list (a dics mapping variable index to
     # a scalar affine function).
-    # TODO: flip these signs a priorie instead of require post processing later
+    # `scalar_affine_terms[x_j]` holds the column `A_{.j}` of the primal
+    # constraint matrix, with the sign it has in the primal model, while every
+    # dual constraint uses `-A_{.j}^T y`.
+    # These terms cannot be negated here: they are also returned for
+    # `_get_dual_objective`, whose entries for variables that are parameters
+    # must keep the primal sign.
     scalar_affine_terms = _get_scalar_affine_terms(
         primal_model,
         primal_dual_map.primal_constraint_data,
@@ -39,8 +44,9 @@ function _add_dual_equality_constraints(
     # Collect affine terms of dual constraints that come from the quadratic
     # part of the primal objective function, and add them into
     # `scalar_affine_terms`.
-    # These terms are added with flipped signs (because the sign will be flipped again).
-    # TODO: unflip these signs
+    # These terms are added with the same sign convention as the terms coming
+    # from the primal constraints, so that all of them can be negated at once
+    # when the dual constraint function is built.
     _add_scalar_affine_terms_from_quad_obj(
         scalar_affine_terms,
         primal_dual_map.primal_var_in_quad_obj_to_dual_slack_var,
@@ -49,8 +55,9 @@ function _add_dual_equality_constraints(
     )
 
     # terms from mixing variables and parameters
-    # These terms are added with flipped signs (because the sign will be flipped again).
-    # TODO: unflip these signs
+    # As above, these are added with the primal sign convention. Note that they
+    # are keyed by the *variable* of each product `parameter * variable`, hence
+    # they never touch the parameter entries read by `_get_dual_objective`.
     _add_scalar_affine_terms_from_quad_params(
         scalar_affine_terms,
         primal_dual_map.primal_parameter_to_dual_parameter,
@@ -66,7 +73,6 @@ function _add_dual_equality_constraints(
         # primal constrained variable.
         # If the dual set is Reals, the constraint is not added, butthe function
         # is cached inthe primal dual map.
-        # TODO: flip these signs a priori instead of requiring post-processing later
         _add_constrained_variable_constraint(
             dual_model,
             primal_model,
@@ -90,14 +96,18 @@ function _add_dual_equality_constraints(
         # these are constraints associated to primal variables that are not
         # treated as constrained variables, that is "free variables" (x \in R)
         # therefore their associated dual constraints are equalities.
+        # The function is `-A_{.j}^T y + a_j` for minimization and
+        # `-A_{.j}^T y - a_j` for maximization, the same convention used by
+        # `_add_constrained_variable_constraint`. Negating both sides here
+        # would be valid for an equality, but it would make the sign of a dual
+        # constraint depend on whether the primal variable is free or
+        # constrained, see
+        # https://github.com/jump-dev/Dualization.jl/issues/70.
         dual_ci = MOI.Utilities.normalize_and_add_constraint(
             dual_model,
             MOI.ScalarAffineFunction(
-                # TODO: flip these two signs bellow to match _add_constrained_variable_constraint
-                # MOI.Utilities.operate_terms(-, scalar_affine_terms[primal_vi]),
-                # sense_change * get(scalar_terms, primal_vi, zero(T))),
-                MOI.Utilities.operate_terms(+, scalar_affine_terms[primal_vi]),
-                -sense_change * get(scalar_terms, primal_vi, zero(T)),
+                MOI.Utilities.operate_terms(-, scalar_affine_terms[primal_vi]),
+                sense_change * get(scalar_terms, primal_vi, zero(T)),
             ),
             MOI.EqualTo(zero(T)),
         )

src/dual_names.jl

@@ -30,7 +30,7 @@ Subject to
 julia> print(dual_model)
 Max dual_var_c
 Subject to
- dual_con_x : dual_var_c = 1
+ dual_con_x : -dual_var_c = -1
  dual_var_c ≥ 0
 ```
 """

test/Tests/test_attributes.jl

@@ -233,7 +233,10 @@ function _test_simple(T, dual_model)
         }(),
     )[]
     @test MOI.get(dual, MOI.VariablePrimalStart(), x) == 1
-    @test MOI.get(dual_model, MOI.ConstraintDualStart(), dual_eq) == -1
+    # The dual constraint of the free variable `x` is `-2y + 0 == 0`, so the
+    # primal start of `x` is stored as the dual start of that constraint with
+    # no sign change, see https://github.com/jump-dev/Dualization.jl/issues/70.
+    @test MOI.get(dual_model, MOI.ConstraintDualStart(), dual_eq) == 1
     # We could set it to zero, but `nothing` should be fine for the solver,
     # let's only revisit if we have a convincing use case
     @test isnothing(MOI.get(dual_model, MOI.ConstraintPrimalStart(), dual_eq))
@@ -252,6 +255,12 @@ function _test_simple(T, dual_model)
 
     MOI.set(dual, MOI.VariablePrimalStart(), vars[], nothing)
     @test isnothing(MOI.get(dual, MOI.VariablePrimalStart(), vars[]))
+
+    # `MOI.VariableName` maps to the `MOI.ConstraintName` of the associated
+    # dual constraint and is passed through unchanged.
+    MOI.set(dual, MOI.VariableName(), x, "x")
+    @test MOI.get(dual, MOI.VariableName(), x) == "x"
+    @test MOI.get(dual_model, MOI.ConstraintName(), dual_eq) == "x"
     return
 end