Le lundi 14 septembre 2009 à 22:47 -0400, houtan a écrit :

I'm sorry, but I was not aware of the coding standards. Before I received this email, I had made some edits to the code using XCode (Mac dev tool). I have since switched to emacs and set it such that only spaces are used. Should I svn commit everything so that when you run the update it will take care of any tabs that may still remain in my local version? NB: though the code compiles, the Expectation operator is not complete.

The basic principles are that: * each SVN commit should add a single feature or fix a single bug, * the associated patch should be readable.

Therefore you should not modify lines which have no relation to the purpose of the commit. And a commit which has a lot of modifications which are only indentation changes is not very readable, since it seems to modify a lot of lines and it is hard to detect what are the "real" changes among the mass of indentation changes.

So in your future commits you should not fix your previous indentation mistakes, except when you functionally change these parts.

In the near future I will run an automatic reindatation program, and this change will be the purpose of a single commit which will *only* consist of indentation changes.

    > you are right, there is a flaw in the specification: oo_ is
    not know to
    > function <fname>_dynamic.m There are two solutions:
    > 1) add steady_state to the list of arguments of this
    function. This is
    > probably more elegant. The drawback is that we increase the
    number of
    > arguments of the function for a rarely used feature
    > 2) call Matlab global variable oo_ from inside
    <fname>_dynamic.m. The
    > preprocessor can add the code only for those models that
    actually use
    > the STEADY_STATE operator. In this case, it will be slower
    than 1), but
    > the code doesn't change for the majority of cases.
    > What do you think, everybody?


    I think I prefer solution 2): this operator is probably not
    used enough
    to warrant the slowdown which would probably be incurred with
    solution
    1).

I have modified the DataTree class such that it contains a bool function containsSteadyStateOperator() and a protected bool steady_state_found, which is initialized to false and set to true in AddSteadyState(). Then, in DynamicModel.cc I write "global oo_;" only if containsSteadyStateOperator() returns true. Is this the solution you had in mind? Do you have any other suggestions?

I think this is ok. A cleaner way would have been to add a recursive method "bool ExprNode::containsSteadyStateOperator()", but I don't know if this is necessary for such a small feature.

Also, regarding eval_opcode for both the STEADY_STATE operator and the EXPECTATION operator, should it throw an EvalException?

The current implementation of UnaryOpNode::eval_opcode() for STEADY_STATE operator looks ok for me: since this method is only used when evaluating the static model, it is logical to return the value inside the operator.

For the EXPECTATION operator, let throw an EvalException, until we decide something else.

Finally, should the derivative of the EXPECTATION operator just return the derivative of the expression (darg2)?

We probably need to discuss the implementation of the EXPECTATION operator.

Since the idea is to create auxiliary variables, my first thought is that it is probably better to create a method on DataTree which would: * walk through the expression tree and detect the expectation operators * create the necessary auxiliary variables * walk through the expression tree and remove the expectation operators by replacing them by these new auxiliary variables

Then we could go through the business of the usual derivation stuff, with no need to add special derivation rules for this operator.

Michel: does this sounds ok to you?

Best,

Hi Houtan

1) You are mentioning DataTree structure. What is it used for? I am currently developing a data tree type structure for passing and holding large number of Dynare parameters inside C++ (including sub-structures).

Is it possible to see your DataTree? I wander if we should try to coordinate our work.

2) Expectational variables such as E_t,t (i.e. E(0)(x)) will be used extensively as part of partial information project and I expect that Sebastien's suggestion to use auxiliary variable would do the trick for them at least.

Best regards

George

----- Original Message ----- From: "Sébastien Villemot" sebastien.villemot@ens.fr To: "List for Dynare developers" dev@dynare.org Sent: Tuesday, September 15, 2009 10:49 AM Subject: Re: [DynareDev] operators

Le lundi 14 septembre 2009 à 22:47 -0400, houtan a écrit :

I'm sorry, but I was not aware of the coding standards. Before I received this email, I had made some edits to the code using XCode (Mac dev tool). I have since switched to emacs and set it such that only spaces are used. Should I svn commit everything so that when you run the update it will take care of any tabs that may still remain in my local version? NB: though the code compiles, the Expectation operator is not complete.

The basic principles are that: * each SVN commit should add a single feature or fix a single bug, * the associated patch should be readable.

Therefore you should not modify lines which have no relation to the purpose of the commit. And a commit which has a lot of modifications which are only indentation changes is not very readable, since it seems to modify a lot of lines and it is hard to detect what are the "real" changes among the mass of indentation changes.

So in your future commits you should not fix your previous indentation mistakes, except when you functionally change these parts.

In the near future I will run an automatic reindatation program, and this change will be the purpose of a single commit which will *only* consist of indentation changes.

    > you are right, there is a flaw in the specification: oo_ is
    not know to
    > function <fname>_dynamic.m There are two solutions:
    > 1) add steady_state to the list of arguments of this
    function. This is
    > probably more elegant. The drawback is that we increase the
    number of
    > arguments of the function for a rarely used feature
    > 2) call Matlab global variable oo_ from inside
    <fname>_dynamic.m. The
    > preprocessor can add the code only for those models that
    actually use
    > the STEADY_STATE operator. In this case, it will be slower
    than 1), but
    > the code doesn't change for the majority of cases.
    > What do you think, everybody?


    I think I prefer solution 2): this operator is probably not
    used enough
    to warrant the slowdown which would probably be incurred with
    solution
    1).

I have modified the DataTree class such that it contains a bool function containsSteadyStateOperator() and a protected bool steady_state_found, which is initialized to false and set to true in AddSteadyState(). Then, in DynamicModel.cc I write "global oo_;" only if containsSteadyStateOperator() returns true. Is this the solution you had in mind? Do you have any other suggestions?

I think this is ok. A cleaner way would have been to add a recursive method "bool ExprNode::containsSteadyStateOperator()", but I don't know if this is necessary for such a small feature.

Also, regarding eval_opcode for both the STEADY_STATE operator and the EXPECTATION operator, should it throw an EvalException?

The current implementation of UnaryOpNode::eval_opcode() for STEADY_STATE operator looks ok for me: since this method is only used when evaluating the static model, it is logical to return the value inside the operator.

For the EXPECTATION operator, let throw an EvalException, until we decide something else.

Finally, should the derivative of the EXPECTATION operator just return the derivative of the expression (darg2)?

We probably need to discuss the implementation of the EXPECTATION operator.

Since the idea is to create auxiliary variables, my first thought is that it is probably better to create a method on DataTree which would: * walk through the expression tree and detect the expectation operators * create the necessary auxiliary variables * walk through the expression tree and remove the expectation operators by replacing them by these new auxiliary variables

Then we could go through the business of the usual derivation stuff, with no need to add special derivation rules for this operator.

Michel: does this sounds ok to you?

Best,

Sébastien Villemot wrote:

Le lundi 14 septembre 2009 à 22:47 -0400, houtan a écrit :

...

I'm sorry, but I was not aware of the coding standards. Before I received this email, I had made some edits to the code using XCode (Mac dev tool). I have since switched to emacs and set it such that only spaces are used. Should I svn commit everything so that when you run the update it will take care of any tabs that may still remain in my local version? NB: though the code compiles, the Expectation operator is not complete.

The basic principles are that:

• each SVN commit should add a single feature or fix a single bug,
• the associated patch should be readable.

Therefore you should not modify lines which have no relation to the purpose of the commit. And a commit which has a lot of modifications which are only indentation changes is not very readable, since it seems to modify a lot of lines and it is hard to detect what are the "real" changes among the mass of indentation changes.

So in your future commits you should not fix your previous indentation mistakes, except when you functionally change these parts.

In the near future I will run an automatic reindatation program, and this change will be the purpose of a single commit which will *only* consist of indentation changes.

...

    > you are right, there is a flaw in the specification: oo_ is
    not know to
    > function <fname>_dynamic.m There are two solutions:
    > 1) add steady_state to the list of arguments of this
    function. This is
    > probably more elegant. The drawback is that we increase the
    number of
    > arguments of the function for a rarely used feature
    > 2) call Matlab global variable oo_ from inside
    <fname>_dynamic.m. The
    > preprocessor can add the code only for those models that
    actually use
    > the STEADY_STATE operator. In this case, it will be slower
    than 1), but
    > the code doesn't change for the majority of cases.
    > What do you think, everybody?


    I think I prefer solution 2): this operator is probably not
    used enough
    to warrant the slowdown which would probably be incurred with
    solution
    1).

I have modified the DataTree class such that it contains a bool function containsSteadyStateOperator() and a protected bool steady_state_found, which is initialized to false and set to true in AddSteadyState(). Then, in DynamicModel.cc I write "global oo_;" only if containsSteadyStateOperator() returns true. Is this the solution you had in mind? Do you have any other suggestions?

I think this is ok. A cleaner way would have been to add a recursive method "bool ExprNode::containsSteadyStateOperator()", but I don't know if this is necessary for such a small feature.

...

Also, regarding eval_opcode for both the STEADY_STATE operator and the EXPECTATION operator, should it throw an EvalException?

The current implementation of UnaryOpNode::eval_opcode() for STEADY_STATE operator looks ok for me: since this method is only used when evaluating the static model, it is logical to return the value inside the operator.

For the EXPECTATION operator, let throw an EvalException, until we decide something else.

...

Finally, should the derivative of the EXPECTATION operator just return the derivative of the expression (darg2)?

We probably need to discuss the implementation of the EXPECTATION operator.

Since the idea is to create auxiliary variables, my first thought is that it is probably better to create a method on DataTree which would:

• walk through the expression tree and detect the expectation operators
• create the necessary auxiliary variables
• walk through the expression tree and remove the expectation operators

by replacing them by these new auxiliary variables

Then we could go through the business of the usual derivation stuff, with no need to add special derivation rules for this operator.

Michel: does this sounds ok to you?

Yes, it is. As we discussed, creating auxiliary variables is also necessary for k order (k > 1) approximation of models with leads on more than one period. So, the mechanism of creating auxiliary variables, should be independent of the expectation operator.

Best

Michel

Best,

Le mercredi 16 septembre 2009 à 20:39 -0400, houtan a écrit :

Just to make sure I understand completely, the reason you proposed the method above over the one outlined earlier (which seemed to replace the EXPECTATION operator as it was encountered, and is outlined below for reference) is that since we are declaring auxiliary variables on the fly, and since we are creating the modfile on the fly as well, we want to be certain that we do not declare a variable for our use (in this case aux*) that the user may have referenced at a later point in their .m file.

This is actually not the main reason for my proposing another solution. But this is a side effect, though.

If this is wrong and all of the symbols have in fact been declared in the SymbolTable by the time the parser begins to create expression trees, would the following be a valid procedure for implementing the Expectation operator?

Following the names EXPECTATION(iArg1)(iArg2),

in NodeID DataTree::AddExpectation(NodeID iArg1, NodeID iArg2)

• Create new auxiliary variable, auxvar(iArg1) via a separate

protected function (using addSymbol, addVariable) that will find a unique variable name

• Add equation: auxvar = iArg2(-iArg1) to the model tree
• return the NodeID of auxvar(iArg1) (as requested

here: http://www.dynare.org/DynareWiki/NewOperators )

If not, what is the advantage of the method proposed earlier over the one contained herein?

Because I want to clearly separate the parsing pass from the computing pass in the preprocessor. In my mind, parsing is only building an expression tree, exactly reflecting what the user has written. So I don't like the idea of doing this on-the-fly. I prefer to clearly separate things in several tasks. For the moment the preprocessor has 4 steps: * parsing * checking (ModFile::checkPass()) * computing (ModFile::computingPass()) * output writing (ModFile::writeOutputFiles())

All the treatment of the expectation operator (adding auxiliary variables and substituting the operator by the new variables) should be done at the beginning of the computing pass. The checking pass may be used to identify the set of expectation operators to be replaced (but only identifying, since it is meant to be "read-only").

Also note that, with Michel, we agreed on the fact that the auxiliary variable names should be meaningful for the user, because she will see it in the output of Dynare. The name needs also to be a valid MATLAB variable name, because we construct output structures with the endogenous names. So for example the auxiliary variable created by expression E(1)(x) could be called E_1_x, which is both meaningful and valid.

If it happens that this name collides with a variable declared by the user, then the preprocessor should exit with an error message, telling the user to use another name in order to avoid confusion (even for his own sake).

Also, why does the NewOperators page say we need to keep a list of new auxiliary variables? This wouldn't be necessary if the idea proposed herein is correct (but certainly would be if we were to follow the method proposed earlier).

It is up to you to decide if you need such a list in the implementation. My impression is that you don't need it, but I may be wrong.

Finally, the NewOperators page states "if the expression has not been encountered in the file, create a new auxiliary variable..." Shouldn't this new auxiliary variable be created, regardless of whether or not the expression has already been encountered?

This sentence was only meaningful in the context of an on-the-fly substitution. It is obsolete in the setting that I have in mind.

Best,

Le mercredi 07 octobre 2009 à 17:29 -0400, houtan a écrit :

I have encountered a couple of problems coding up the Expectation operator which would be alleviated if you could answer the following questions:

1. In DataTree::AddVariable(int symb_id, int lag), why is there an

"assert(lag==0);" at the beginning of this file and how can we add variables to the datatree with leads/lags?

This is the default implementation of that method, used in StaticModel, to be sure that we have only variables at the current period. Since this method is declared virtual, it is overloaded in DynamicModel::AddVariable() by another implementation which allows leads/lags.

2. I am a bit confused as to how negative numerial constants should be

stored in the datatree. Should they be stored as a unaryOpNode of type oUminus pointing to a positive numConstNode (as is the case for MinusOne and MinusIninity)? Or should they be stored as a potentially negative number as in ParsingDriver::add_constant(string *constant)?

First case:

UnaryOpNode(oUminus) -> NumConstNode(>=0)

3. Is there a guideline for which types of potential errors to handle

nicely (by printing a message to cerr and exiting) and which ones to simply allow to pass by? Say, for example (as I have not coded this fully yet), I would like to look up an oExpectation BinaryOpNode in the subst_table when replacing the oExpectation BinaryOpNode with a Variable node pointing to the associated auxiliary variable. This auxiliary variable would be provided if I had correctly inserted everything into the subst_table. If, on the other hand, there is a bug in my code, it will not return this and will break during processing. Though this will be tested for after the Expectation operator has been implemented, you can see that there are many such situations that could potentially arise. Hence, I was wondering if you have any advice as to when one should test for errors in processing (and print appropriate error messages) and when one should assume that everything has worked correctly. I hope this is clear and that I have not been too long-winded.

The best practice is to use assert() for debugging purposes: it emphasizes the conditions which are supposed to be met at some point in the code, so this enhances the readability of the code. The idea is that when the condition is not satisfied, there is a bug in the code: then the assert() will display an error message with the source filename and line number, and exit the program.

The use of cerr followed by an exit is appropriate for messages which indicate an error coming from the user.

However I don't always follow this practice and use cerr/exit for debugging purposes at some places, but this is something which we should better avoid.

Best,