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,