From 55cd8da9b865eaf3b1a52994b3c6996fc4eaeff3 Mon Sep 17 00:00:00 2001
From: Gleb Belov <gleb@ampl.com>
Date: Mon, 28 Oct 2024 11:53:15 +1100
Subject: [PATCH] MP2NL:explicify logical args in algebraic expr #237

---
 include/mp/flat/constr_2_expr.h          | 33 ++++++++++++------------
 include/mp/flat/nl_expr/model_api_base.h |  2 +-
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/mp/flat/constr_2_expr.h b/include/mp/flat/constr_2_expr.h
index e00ea9323..6aebc1478 100644
--- a/include/mp/flat/constr_2_expr.h
+++ b/include/mp/flat/constr_2_expr.h
@@ -75,7 +75,7 @@ class Constraints2Expr {
   void ConsiderMarkingArguments(
       const Con& con, int i, ExpressionAcceptanceLevel eal) {
     bool fMarkArgs = false;
-    if (con.HasResultVar())    // func cons: non-accepted ones by default
+    if (con.HasResultVar())    // func cons: those not accepted as expr
       fMarkArgs = (ExpressionAcceptanceLevel::NotAccepted==eal);
     else
       fMarkArgs = true;        // static cons: all non-algebraic by default
@@ -128,6 +128,7 @@ class Constraints2Expr {
     // Otherwise it's a flat con.
     if (ExpressionAcceptanceLevel::NotAccepted != eal) {
       if (1==stage_cvt2expr_) {
+        HandleLogicalArgs(con, i);         // expicify logical args
         if (!con.GetConstraint().GetBody().is_variable()) {  // already a variable
           ConvertConditionalConLHS(con, i);
           return true;
@@ -149,6 +150,9 @@ class Constraints2Expr {
       const FuncCon& con, int i,
       ConstraintAcceptanceLevel , ExpressionAcceptanceLevel eal) {
     if (ExpressionAcceptanceLevel::NotAccepted != eal) {     // going into an expr
+      if constexpr (std::is_base_of_v<NumericFunctionalConstraintTraits, FuncCon>)
+        if (1==stage_cvt2expr_)
+          HandleLogicalArgs(con, i);
       if (2==stage_cvt2expr_)
         ConsiderExplicifyingExpression(con, i);
     }
@@ -159,6 +163,8 @@ class Constraints2Expr {
   bool ConvertWithExpressions(
       const LinearFunctionalConstraint& con, int i,
       ConstraintAcceptanceLevel , ExpressionAcceptanceLevel eal) {
+    if (1==stage_cvt2expr_)
+      HandleLogicalArgs(con, i);         // explicify logical args
     if (2==stage_cvt2expr_) {
       return ConsiderExplicifyingAlgebraic(con, i);
     }
@@ -169,6 +175,8 @@ class Constraints2Expr {
   bool ConvertWithExpressions(
       const QuadraticFunctionalConstraint& con, int i,
       ConstraintAcceptanceLevel , ExpressionAcceptanceLevel eal) {
+    if (1==stage_cvt2expr_)
+      HandleLogicalArgs(con, i);         // explicify logical args
     if (2==stage_cvt2expr_) {
       return ConsiderExplicifyingAlgebraic(con, i);
     }
@@ -183,6 +191,8 @@ class Constraints2Expr {
       const ComplementarityConstraint<Expr>& con,
       int i,
       ConstraintAcceptanceLevel , ExpressionAcceptanceLevel ) {
+    if (1==stage_cvt2expr_)
+      HandleLogicalArgs(con, i);         // explicify logical args
     if (false                      // TODO check acc for NLCompl
         && 1==stage_cvt2expr_
         && !con.GetExpression().is_variable()) {             // already a variable
@@ -276,6 +286,8 @@ class Constraints2Expr {
   void ConvertObjectivesWithExpressions() {
     auto& objs = MPD( get_objectives() );
     for (size_t iobj=0; iobj<objs.size(); ++iobj) {
+      HandleLogicalArgs(objs[iobj].GetLinTerms(), iobj);
+      HandleLogicalArgs(objs[iobj].GetQPTerms(), iobj);
       Convert1ObjWithExpressions(iobj, objs[iobj]);
     }
   }
@@ -375,28 +387,17 @@ class Constraints2Expr {
     return false;
   }
 
-  /// Handle logical expression in a linear con
+  /// Handle logical expression in an algebraic con
   /// @return whether to remove the original \a con.
-  template <class RhsOrRange>
-  bool HandleLogicalArgs(
-      const AlgebraicConstraint<LinTerms, RhsOrRange>& con, int i) {
-    if (false && HandleLogicalArgs_SpecialCases(con, i))
-      return true;
-    VisitArguments(con, MarkVarIfLogical_);          // Mark as proper vars
-    return false;                                    // don't remove immediately
-  }
-
-  /// Handle logical expression in a quadratic con
-  template <class RhsOrRange>
-  bool HandleLogicalArgs(
-      const AlgebraicConstraint<QuadAndLinTerms, RhsOrRange>& con, int ) {
+  template <class Con>
+  bool HandleLogicalArgs(const Con& con, int ) {
     VisitArguments(con, MarkVarIfLogical_);          // Mark as proper vars
     return false;                                    // don't remove immediately
   }
 
   /// Special linear cases.
   /// Not doing any more because these simplifications
-  /// interfere withg result variable marking.
+  /// interfere with result variable marking.
   /// These simplifications are general presolve
   /// and should better have been done in normal conversion stage.
   /// @todo atleast, atmost, exactly
diff --git a/include/mp/flat/nl_expr/model_api_base.h b/include/mp/flat/nl_expr/model_api_base.h
index cdb5366d7..ad911c36e 100644
--- a/include/mp/flat/nl_expr/model_api_base.h
+++ b/include/mp/flat/nl_expr/model_api_base.h
@@ -324,7 +324,7 @@ class BasicExprModelAPI
     if (IsVarProper(i_expr)) {
       is_init_expr_retrieved_[i_expr] = true;             // is being explicified
       Expr result;
-      get_init_expr_(i_expr, &result);
+      get_init_expr_(i_expr, &result);    // TODO we are not caching them.
       return result;
     }
     return GetInitExpression(i_expr);                     // standard case