From a97a41f6c03c60c0b628d2a9caceec8e182b6222 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Villemot?= <sebastien@dynare.org>
Date: Thu, 21 Jul 2022 17:22:08 +0200
Subject: [PATCH] Bugfix with temporary terms in block+bytecode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There were actually two distinct bugs, leading to incorrect results in some
corner cases:

– in the “evaluate” mode of the bytecode MEX, the temporary terms of the
  derivatives “evaluate” blocks were not evaluated at runtime; but these
  temporary terms may be needed for residuals of subsequent blocks;

– when the bytecode MEX was only computing residuals of the model (and not 1st
  order derivatives), the temporary terms of the derivatives were not evaluated
  at runtime; but these temporary terms may be needed for residuals of subsequent
  blocks.
---
 src/ModelTree.hh | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/ModelTree.hh b/src/ModelTree.hh
index 7c0a5746..bcb4a6d8 100644
--- a/src/ModelTree.hh
+++ b/src/ModelTree.hh
@@ -1303,15 +1303,18 @@ ModelTree::writeBlockBytecodeHelper(BytecodeWriter &code_file, int block) const
           code_file << FBINARY_{BinaryOpcode::minus} << FSTPR_{i - block_recursive};
         }
     }
-  code_file << FENDEQU_{};
 
-  /* If the block is not of type “evaluate backward/forward”, then we write
-     the temporary terms for derivatives at this point, i.e. before the
-     JMPIFEVAL, because they will be needed in both “simulate” and
-     “evaluate” modes. */
-  if (simulation_type != BlockSimulationType::evaluateBackward
-      && simulation_type != BlockSimulationType::evaluateForward)
-    write_eq_tt(blocks[block].size);
+  /* Write temporary terms for derivatives. This is done before FENDEQU,
+     because residuals of a subsequent block may depend on temporary terms for
+     the derivatives of the present block.
+
+     Also note that in the case of “evaluate” blocks, derivatives are not
+     computed in the “evaluate” mode; still their temporary terms must be
+     computed even in that mode, because for the same reason as above they may
+     be needed in subsequent blocks. */
+  write_eq_tt(blocks[block].size);
+
+  code_file << FENDEQU_{};
 
   // Get the current code_file position and jump if evaluating
   int pos_jmpifeval {code_file.getInstructionCounter()};
@@ -1411,13 +1414,6 @@ ModelTree::writeBlockBytecodeHelper(BytecodeWriter &code_file, int block) const
   // Update jump offset for previous JMPIFEVAL
   code_file.overwriteInstruction(pos_jmpifeval, FJMPIFEVAL_{pos_jmp-pos_jmpifeval});
 
-  /* If the block is of type “evaluate backward/forward”, then write the
-     temporary terms for derivatives at this point, because they have not
-     been written before the JMPIFEVAL. */
-  if (simulation_type == BlockSimulationType::evaluateBackward
-      || simulation_type == BlockSimulationType::evaluateForward)
-    write_eq_tt(blocks[block].size);
-
   // Write the derivatives for the “evaluate” mode
   for (const auto &[indices, d] : blocks_derivatives[block])
     {
-- 
GitLab