Marking: only relevant expressions #237

Make sure we only mark active expressions (not redefined ones). Changed the marking logic to start with all vars - mark expressions - mark arguments which need to be vars

include/mp/flat/constr_2_expr.h

@@ -13,23 +13,41 @@ namespace mp {
 template <class Impl>
 class Constraints2Expr {
 public:
+  /// Convert some functional constraints to expressions
+  void Convert2NL() {
+    MPD( MarkExpressions() );
+    MPD( GetModel() ).ConvertWithExpressions(*(Impl*)this);
+  }
+
+  /// Mark which functional constraints to be used as expressions,
+  /// vs assigning their result to a variable
+  void MarkExpressions() {
+    MPD( MarkAllResultVarsAsVars() );
+    MPD( GetModel() ).MarkExprResultVars(*(Impl*)this);
+    MPD( GetModel() ).MarkArguments(*(Impl*)this);
+  }
 
-  /// Consider marking the result and argument variables as
-  /// "explicit variables" (not expressions)
+  /// Consider marking the result variables as
+  /// possible expressions
   template <class Con>
-  void ConsiderMarkingResultAndArgVars(
-      const Con& con, int i, ExpressionAcceptanceLevel eal) {
+  void ConsiderMarkingResultVar(
+      const Con& con, int , ExpressionAcceptanceLevel eal) {
     if (con.HasResultVar()) {         // A functional constraint
-      if (ExpressionAcceptanceLevel::NotAccepted==eal) {
-        MPD( MarkAsResultVar(con.GetResultVar()) );
+      // Check that it will be expression, but possibly with a dedicated result variable
+      if (ExpressionAcceptanceLevel::NotAccepted!=eal) {
+        assert(                     // Check: the result var has \a con as the init expr
+            MPD( template GetInitExpressionOfType<Con>(con.GetResultVar()) )
+                       == &con);
+        MPD( MarkAsExpression(con.GetResultVar()) );   // can be changed later
       }
-    }                          // Any constraint
-    MPD( ConsiderMarkingArgumentsAsVars(con, i, eal) );
+    }
   }
 
-  /// Generic request to consider marking arguments
+  /// Consider marking the argument variables as
+  /// "explicit variables" (not expressions.)
+  /// Generic request.
   template <class Con>
-  void ConsiderMarkingArgumentsAsVars(
+  void ConsiderMarkingArguments(
       const Con& con, int i, ExpressionAcceptanceLevel eal) {
     bool fMarkArgs = false;
     if (con.HasResultVar())    // func cons: non-accepted ones by default

include/mp/flat/constr_base.h

@@ -56,8 +56,8 @@ class BasicConstraint {
   void SetContext(Context ) const { }
   /// Add (merge) context, if meaningful
   void AddContext(Context ) const { }
-    /// Has result var (is functional)?
-    bool HasResultVar() const { return false; }
+  /// Has result var (is functional)?
+  bool HasResultVar() const { return false; }
   /// For functional constraints, result variable index
   int GetResultVar() const { return -1; }
   /// Compute violation

include/mp/flat/constr_keeper.h

@@ -380,8 +380,11 @@ class BasicConstraintKeeper {
   /// @return whether any converted
   virtual bool ConvertAllNewWith(BasicFlatConverter& cvt) = 0;
 
-  /// Mark whether to keep result vars
-  virtual void MarkExprsForResultVars(BasicFlatConverter& cvt) = 0;
+  /// Mark whether we could keep result vars
+  virtual void MarkExprResultVars(BasicFlatConverter& cvt) = 0;
+
+  /// Then, mark flat constraint arguments as vars
+  virtual void MarkArguments(BasicFlatConverter& cvt) = 0;
 
   /// Convert to use expressions
   virtual void ConvertWithExpressions(BasicFlatConverter& cvt) = 0;
@@ -730,12 +733,19 @@ class ConstraintKeeper final
     return false;
   }
 
-  /// Mark whether to keep result vars
-  void MarkExprsForResultVars(BasicFlatConverter& cvt) override {
+  /// Mark whether we could result vars of functional constraints
+  /// as vars, vs using these constraints as expressions
+  void MarkExprResultVars(BasicFlatConverter& cvt) override {
     assert(&cvt == &GetConverter());         // Using the same Converter
     DoMarkForResultVars();
   }
 
+  /// Then, mark arguments of flat constraints as proper vars
+  void MarkArguments(BasicFlatConverter& cvt) override {
+    assert(&cvt == &GetConverter());         // Using the same Converter
+    DoMarkForArguments();
+  }
+
   /// Convert to use expressions
   void ConvertWithExpressions(BasicFlatConverter& cvt) override {
     assert(&cvt == &GetConverter());         // Using the same Converter
@@ -908,7 +918,19 @@ class ConstraintKeeper final
       const auto& cnt = cons_[i];
       if (!cnt.IsBridged()) {      // Delegate actual logic to Converter
         const auto& con = cnt.GetCon();
-        GetConverter().ConsiderMarkingResultAndArgVars(con, i, eal);
+        GetConverter().ConsiderMarkingResultVar(con, i, eal);
+      }
+    }
+  }
+
+  void DoMarkForArguments() {
+    const auto eal        // expr only
+        = GetChosenAcceptanceLevelEXPR();
+    for (int i=0; i< (int)cons_.size(); ++i) {
+      const auto& cnt = cons_[i];
+      if (!cnt.IsBridged()) {      // Delegate actual logic to Converter
+        const auto& con = cnt.GetCon();
+        GetConverter().ConsiderMarkingArguments(con, i, eal);
       }
     }
   }
@@ -1283,10 +1305,16 @@ class ConstraintManager {
     } while (any_converted);
   }
 
-  /// Mark which expressions should stay as FuncCons or just have a result variable
-  void MarkExprsForResultVars(BasicFlatConverter& cvt) {
+  /// Mark which func cons can be expressions
+  void MarkExprResultVars(BasicFlatConverter& cvt) {
+    for (auto& ck: con_keepers_)
+      ck.second.MarkExprResultVars(cvt);
+  }
+
+  /// Then, mark arguments of flat cons as vars
+  void MarkArguments(BasicFlatConverter& cvt) {
     for (auto& ck: con_keepers_)
-      ck.second.MarkExprsForResultVars(cvt);
+      ck.second.MarkArguments(cvt);
   }
 
   /// Convert to expression-based model

include/mp/flat/converter.h

@@ -277,12 +277,6 @@ class FlatConverter :
            ExpressionAcceptanceLevel::Recommended
                == ModelAPI::ExpressionInterfaceAcceptanceLevel(); }
 
-  /// Convert some functional constraints to expressions
-  void Convert2NL() {
-    GetModel().MarkExprsForResultVars(*this);
-    GetModel().ConvertWithExpressions(*this);
-  }
-
   /// Finish exporting the reformulation graph
   void CloseGraphExporter() {
     value_presolver_.FinishExportingLinkEntries();

include/mp/flat/converter_model.h

@@ -228,18 +228,25 @@ class FlatModel
     var_ub_[v] = u;
   }
 
+  /// To be called first
+  void MarkAllResultVarsAsVars() {
+    var_result_.clear();
+    var_result_.resize(num_vars(), true);
+  }
+
   /// Mark as an explicit result variable
   void MarkAsResultVar(int v) {
-    if (var_result_.size()<=v)
-      var_result_.resize(num_vars());
-    var_result_[v] = true;
+    var_result_.at(v) = true;
+  }
+
+  /// Mark as a proper expression
+  void MarkAsExpression(int v) {
+    var_result_.at(v) = false;
   }
 
   /// Is the variable an explicit result var?
   bool IsResultVar(int v) const {
-    if (var_result_.size()<=v)
-      var_result_.resize(num_vars());
-    return var_result_[v];
+    return var_result_.at(v);
   }
 
   ///////////////////////////// OBJECTIVES ////////////////////////////

include/mp/flat/redef/MIP/ifthenelse.h

@@ -36,10 +36,11 @@ class IfThenElseConverter_MIP :
     assert((GetMC().is_fixed(args[1]) && GetMC().is_fixed(args[2])));
     const double const1 = GetMC().fixed_value(args[1]);
     const double const2 = GetMC().fixed_value(args[2]);
-    /// Obtain result variable via map
+    // Obtain result variable via map
     int var_res_lin = GetMC().AssignResultVar2Args(
           LinearFunctionalConstraint(
             { {{const1-const2}, {args[0]}}, const2 } ));
+    // TODO just redefine init expr
     GetMC().AddConstraint(LinConEQ{
                             { {-1.0, 1.0},
                               {itc.GetResultVar(), var_res_lin} },