From 15d7432105578c749cd5a81b2ab09fe8bf01d1b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Villemot?= <sebastien@dynare.org>
Date: Wed, 21 Jul 2021 16:16:39 +0200
Subject: [PATCH] =?UTF-8?q?Occbin:=20add=20more=20sanity=20checks=20on=20e?=
 =?UTF-8?q?xpressions=20in=20=E2=80=9Coccbin=5Fconstraints=E2=80=9D=20bloc?=
 =?UTF-8?q?k?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

— forbid leads and lags
— forbid expectation operators

This is implemented by moving the occbin_constraints expression to a separate
DataTree. As a side-effect, this removes the spurious non-linearity warning in
a stochastic context (because we were introducing inequalities and abs()
operators in the main DynamicModel tree).
---
 src/ComputingTasks.cc |  6 ++---
 src/ComputingTasks.hh |  4 ++--
 src/ParsingDriver.cc  | 55 +++++++++++++++++++++++++++++++------------
 src/ParsingDriver.hh  |  3 +++
 4 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/src/ComputingTasks.cc b/src/ComputingTasks.cc
index 5407d0ac..98160e51 100644
--- a/src/ComputingTasks.cc
+++ b/src/ComputingTasks.cc
@@ -5235,9 +5235,9 @@ MatchedMomentsStatement::writeJsonOutput(ostream &output) const
   output << "]}" << endl;
 }
 
-OccbinConstraintsStatement::OccbinConstraintsStatement(const SymbolTable &symbol_table_arg,
+OccbinConstraintsStatement::OccbinConstraintsStatement(const DataTree &data_tree_arg,
                                                        const vector<tuple<string, BinaryOpNode *, BinaryOpNode *, expr_t, expr_t>> constraints_arg)
-  : symbol_table{symbol_table_arg}, constraints{constraints_arg}
+  : data_tree{data_tree_arg}, constraints{constraints_arg}
 {
 }
 
@@ -5263,7 +5263,7 @@ OccbinConstraintsStatement::writeOutput(ostream &output, const string &basename,
   output << "M_.occbin.constraint_nbr = " << constraints.size() << ';' << endl
          << "M_.occbin.pswitch = [" << endl;
   for (const auto &[name, bind, relax, error_bind, error_relax] : constraints)
-    output << symbol_table.getTypeSpecificID(ParsingDriver::buildOccbinBindParamName(name)) + 1 << ' ';
+    output << data_tree.symbol_table.getTypeSpecificID(ParsingDriver::buildOccbinBindParamName(name)) + 1 << ' ';
   output << "];" << endl
          << "options_.occbin = struct();" << endl
          << "options_.occbin = occbin.set_default_options(options_.occbin, M_);" << endl
diff --git a/src/ComputingTasks.hh b/src/ComputingTasks.hh
index bf125721..91dd83d0 100644
--- a/src/ComputingTasks.hh
+++ b/src/ComputingTasks.hh
@@ -1257,11 +1257,11 @@ public:
 class OccbinConstraintsStatement : public Statement
 {
 private:
-  const SymbolTable &symbol_table;
+  DataTree data_tree;
 public:
   // The tuple is (name, bind, relax, error_bind, error_relax) (where relax and error_{bind,relax} can be nullptr)
   const vector<tuple<string, BinaryOpNode *, BinaryOpNode *, expr_t, expr_t>> constraints;
-  OccbinConstraintsStatement(const SymbolTable &symbol_table_arg,
+  OccbinConstraintsStatement(const DataTree &data_tree_arg,
                              const vector<tuple<string, BinaryOpNode *, BinaryOpNode *, expr_t, expr_t>> constraints_arg);
   void checkPass(ModFileStructure &mod_file_struct, WarningConsolidation &warnings) override;
   void writeOutput(ostream &output, const string &basename, bool minimal_workspace) const override;
diff --git a/src/ParsingDriver.cc b/src/ParsingDriver.cc
index f9282447..591a0763 100644
--- a/src/ParsingDriver.cc
+++ b/src/ParsingDriver.cc
@@ -391,14 +391,30 @@ ParsingDriver::add_model_variable(int symb_id, int lag)
   if (type == SymbolType::modelLocalVariable && lag != 0)
     error("Model local variable " + mod_file->symbol_table.getName(symb_id) + " cannot be given a lead or a lag.");
 
-  if (dynamic_cast<StaticModel *>(model_tree) && lag != 0)
-    error("Leads and lags on variables are forbidden in 'planner_objective'.");
+  if (data_tree == planner_objective.get())
+    {
+      if (lag != 0)
+        error("Leads and lags on variables are forbidden in 'planner_objective'.");
+
+      if (type == SymbolType::modelLocalVariable)
+        error("Model local variable " + mod_file->symbol_table.getName(symb_id) + " cannot be used in 'planner_objective'.");
+    }
 
-  if (dynamic_cast<StaticModel *>(model_tree) && type == SymbolType::modelLocalVariable)
-    error("Model local variable " + mod_file->symbol_table.getName(symb_id) + " cannot be used in 'planner_objective'.");
+  if (data_tree == occbin_constraints_tree.get())
+    {
+      if (lag != 0)
+        error("Leads and lags on variables are forbidden in 'occbin_constraints'. Note that you can achieve the same effect by introducing an auxiliary variable in the model.");
+
+      if (type == SymbolType::modelLocalVariable)
+        error("Model local variable " + mod_file->symbol_table.getName(symb_id) + " cannot be used in 'occbin_constraints'.");
+
+      if (type == SymbolType::exogenous || type == SymbolType::exogenousDet)
+        error("Exogenous variable " + mod_file->symbol_table.getName(symb_id) + " cannot be used in 'occbin_constraints'.");
+    }
 
   // It makes sense to allow a lead/lag on parameters: during steady state calibration, endogenous and parameters can be swapped
-  return model_tree->AddVariable(symb_id, lag);
+  // NB: we use data_tree here, to avoid a crash in the occbin_constraints case
+  return data_tree->AddVariable(symb_id, lag);
 }
 
 expr_t
@@ -2530,18 +2546,27 @@ ParsingDriver::add_power(expr_t arg1, expr_t arg2)
 expr_t
 ParsingDriver::add_expectation(const string &arg1, expr_t arg2)
 {
+  if (data_tree == occbin_constraints_tree.get())
+    error("The 'expectation' operator is forbidden in 'occbin_constraints'.");
+
   return data_tree->AddExpectation(stoi(arg1), arg2);
 }
 
 expr_t
 ParsingDriver::add_var_expectation(const string &model_name)
 {
+  if (data_tree == occbin_constraints_tree.get())
+    error("The 'var_expectation' operator is forbidden in 'occbin_constraints'.");
+
   return data_tree->AddVarExpectation(model_name);
 }
 
 expr_t
 ParsingDriver::add_pac_expectation(const string &var_model_name)
 {
+  if (data_tree == occbin_constraints_tree.get())
+    error("The 'var_expectation' operator is forbidden in 'occbin_constraints'.");
+
   return data_tree->AddPacExpectation(var_model_name);
 }
 
@@ -3450,7 +3475,15 @@ ParsingDriver::end_matched_moments(const vector<expr_t> &moments)
 void
 ParsingDriver::begin_occbin_constraints()
 {
-  set_current_data_tree(&mod_file->dynamic_model);
+  /* We use a separate data tree, because we will add inequality constraints,
+     and those would trigger the non-linearity warning in a stochastic context
+     if added to the main DynamicModel tree. It also simplifies the
+     enforcement of various constraints at parsing time. */
+  occbin_constraints_tree = make_unique<DataTree>(mod_file->symbol_table,
+                                                  mod_file->num_constants,
+                                                  mod_file->external_functions_table,
+                                                  false);
+  set_current_data_tree(occbin_constraints_tree.get());
 }
 
 void
@@ -3464,17 +3497,9 @@ ParsingDriver::end_occbin_constraints(const vector<tuple<string, BinaryOpNode *,
         error("No equation has been declared for regime '" + name + "'");
       if (!bind)
         error("The 'bind' expression is missing in constraint '" + name + "'");
-      if (bind->hasExogenous())
-        error("Exogenous variables are not allowed in the context of the 'bind' expression");
-      if (relax && relax->hasExogenous())
-        error("Exogenous variables are not allowed in the context of the 'relax' expression");
-      if (error_bind && error_bind->hasExogenous())
-        error("Exogenous variables are not allowed in the context of the 'error_bind' expression");
-      if (error_relax && error_relax->hasExogenous())
-        error("Exogenous variables are not allowed in the context of the 'error_relax' expression");
     }
 
-  mod_file->addStatement(make_unique<OccbinConstraintsStatement>(mod_file->symbol_table, constraints));
+  mod_file->addStatement(make_unique<OccbinConstraintsStatement>(*occbin_constraints_tree, constraints));
 
   reset_data_tree();
 }
diff --git a/src/ParsingDriver.hh b/src/ParsingDriver.hh
index 08bf8c0c..f77ccc85 100644
--- a/src/ParsingDriver.hh
+++ b/src/ParsingDriver.hh
@@ -117,6 +117,9 @@ private:
   //! Temporary store for the planner objective
   unique_ptr<StaticModel> planner_objective;
 
+  //! Temporary store for the occbin_constraints statement
+  unique_ptr<DataTree> occbin_constraints_tree;
+
   //! The data tree in which to add expressions currently parsed
   /*! The object pointed to is not owned by the parsing driver. It is essentially a
       reference. */
-- 
GitLab