Unary ops substitution: bugfix, actually restrict the substitution to the chosen equations

By default, the preprocessor is supposed to only do the “unary ops”
transformation in the equations of VAR/PAC/trend component models.

However, the implementation was slightly different so far. It would detect
candidates to this transformation in the chosen equations, but it would then
perform the substitution in *all* equations.

This could lead for crashes, for example if the chosen equation contains
log(X(-1)), but another (non-chosen) equation has log(X(-2)). Then this latter
expression, even though it belongs to the same lag-equivalence class, is not
properly handled, causing a segfault.

Also do a few related cosmetic changes.
issue#70
Sébastien Villemot 2020-07-08 12:13:35 +02:00
parent 59bda04d8b
commit 5b80a4db59
No known key found for this signature in database
GPG Key ID: 2CECE9350ECEBE4A
2 changed files with 10 additions and 9 deletions

View File

@ -5677,9 +5677,8 @@ DynamicModel::substituteUnaryOps(const vector<int> &eqnumbers)
set<int> used_local_vars;
for (int eqnumber : eqnumbers)
equations[eqnumber]->collectVariables(SymbolType::modelLocalVariable, used_local_vars);
for (auto &it : local_variables_table)
if (used_local_vars.find(it.first) != used_local_vars.end())
it.second->findUnaryOpNodesForAuxVarCreation(nodes);
for (int mlv : used_local_vars)
local_variables_table[mlv]->findUnaryOpNodesForAuxVarCreation(nodes);
// Mark unary ops to be substituted in selected equations
for (int eqnumber : eqnumbers)
@ -5687,16 +5686,16 @@ DynamicModel::substituteUnaryOps(const vector<int> &eqnumbers)
// Substitute in model local variables
vector<BinaryOpNode *> neweqs;
for (auto &it : local_variables_table)
it.second = it.second->substituteUnaryOpNodes(nodes, subst_table, neweqs);
for (int mlv : used_local_vars)
local_variables_table[mlv] = local_variables_table[mlv]->substituteUnaryOpNodes(nodes, subst_table, neweqs);
// Substitute in equations
for (auto &equation : equations)
for (int eq : eqnumbers)
{
auto substeq = dynamic_cast<BinaryOpNode *>(equation->
auto substeq = dynamic_cast<BinaryOpNode *>(equations[eq]->
substituteUnaryOpNodes(nodes, subst_table, neweqs));
assert(substeq);
equation = substeq;
equations[eq] = substeq;
}
// Add new equations

View File

@ -3530,7 +3530,9 @@ UnaryOpNode::substituteUnaryOpNodes(const lag_equivalence_table_t &nodes, subst_
else
subst_table[rit->second] = dynamic_cast<VariableNode *>(aux_var->decreaseLeadsLags(base_index - rit->first));
return const_cast<VariableNode *>(subst_table.find(this)->second);
assert(subst_table.find(this) != subst_table.end());
return const_cast<VariableNode *>(subst_table.at(this));
}
expr_t