Moving to the dev list....
In a conversation with Michel, we decided to rewrite the computation of temporary variables within the preprocessor. reference_count in ModelTree.cc:computeTemporaryTerms will be changed to a map<expr_t, pair<int,int>>, with the second int containing a 0,1,2,3 depending on whether the expression was first encountered in the equations, first, second, or third derivatives. Then when a temporary term is created, it will be inserted into the temporary_terms structure associated with the equations, first, second, or third derivatives. The union of these structures should be equivalent to the current temporary_terms structure and to avoid rewriting large swaths of the code, we can reconstitute temporary_terms from these parts.
Stephane, I have made a bug fix but am first going to revert the change from yesterday and move to a branch and do the temporary term construction there.
Best, Houtan
On 09/02/2015 02:58 PM, Stéphane Adjemian wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
I have the impression that there is a misunderstanding here. I was not as ambitious as Michel seems to have understood. I need first to check what Houtan has done for julia...
Houtan, did you fix the bug you introduced yesterday ?
Best, Stéphane.
Stéphane Adjemian Université du Maine www.dynare.org/stepan
On 02/09/2015 14:20, Michel Juillard wrote:
It is more than a matter to package temporary variables in if blocks: depending whether an equation uses or doens't use temporary variables, it will be written differently.
My feeling is that such a level of optimization is too high for the Matlab implementation. If the user worries so much about efficiency, he/she should switch to bytecode or dll
Best
Michel Houtan Bastani writes:
Sorry for the late reply, I was at the dentist's then lunch.
As Michel points out, the API change is due to the way in which temporary variables are computed in the preprocessor: when we pass through the equations to compute the temporary variables for the residuals, an expression may not have a cost high enough to warrant a temporary variable. If that expression is seen again in the first derivatives and the cost is now high enough, it will be substituted at this point (the cost changes because it is a function of the expression being evaluated and the number of times that expression has been encountered).
An API change is not absolutely necessary as we can include these temporary variables in if statements before assigning g1, g2, and g3. The thing to note is that certain temporary variables will be computed up to 4 times (residuals, g1, g2, g3), which is the case for what you wanted in julia as well.
I agree that, clearly, we should test the speed and correctness of this change before pushing it to master. For know, I'll make the bug fix so that this works in julia, leaving matlab unaffected. I guess before we could test this in matlab, we'd need to revamp the test system to track timing and verify the correctness of the results...
Best, Houtan
On 09/02/2015 12:21 PM, Michel Juillard wrote:
.... In that case, the residual formulas would be written differently depending if one computes only residuals or residuals and first derivatives
Stéphane Adjemian writes:
We do not need to change the API. We just have to define the temporary variables when we need them, ie not on top of the dynamic and static files.
Stéphane.
Stéphane Adjemian Université du Maine www.dynare.org/stepan
On 02/09/2015 10:54, Michel Juillard wrote:
> It would be better to do a test in Matlab of the benefit > of using different sets of temporary variables for > residuals, and successive derivatives before starting > implementing it for Matlab. If we change the current API > for <modfile>_dynamic functions, we will need to go over > all the Matlab files calling this function. > > Best > > Michel > > Houtan Bastani writes: > >> OK, I found the bug. It's something quite simple. >> >> The question now remains: is the separation of >> temporary variables something that we want to bring >> into the Matlab side of things as well? Your answer >> will dictate the change I make. >> >> Michel, just to be clear, on the Julia side of things, >> Stephane wants to separate the temporary variables for >> the various dynamic! functions such that only those >> temporary variables that are used in the calculation of >> the residuals are printed in the dynamic function that >> calculates the residuals; only those temporary >> variables that are used in the calculation of the >> residuals and the jacobian are used in the dynamic >> function that calculates the residuals and the jacobian >> and so on... >> >> I guess in Matlab this would entail having four dynamic >> functions with different calling structures. NB That >> the change I make, for technical reasons that I'd be >> happy to go over on the phone as it's a bit too subtle >> to explain by email, does not contain the same number >> of temporary variables as the current version, but >> since what is declared a temporary variable is rather >> arbitrary, I did not think this mattered. What does >> matter most, in the end, is which method is most >> efficient so if we go with this new method, clearly >> we'd have to test how quickly the functions worked >> before committing it to master. >> >> Best, Houtan >> >> >> On 09/02/2015 09:15 AM, Houtan Bastani wrote: >>> >>> I was too eager to push the temporary variables >>> change last night. I'll revert it, fix it, and push >>> it once I've tested it against the test suite. >>> >>> Best, Houtan >>> >>> On 09/01/2015 09:39 PM, Dynare Robot wrote: >>>> eb2890d1f58f2285306821c85fa631b865fb80b6 >>>> preprocessor: julia: print only those temporary >>>> vars needed for the computation of (residuals, g1, >>>> g2, g3) in the respective dynamic! and static! >>>> functions >>>> >>>> Dynare failed to compile or MATLAB testsuite failed >>>> to run >>>> >>>> Dynare failed to compile or Octave testsuite failed >>>> to run >>>> >>>> A full log can be found at >>>> http://www.dynare.org/testsuite/master >>>> _______________________________________________ Dev >>>> mailing list Dev@dynare.org >>>> https://www.dynare.org/cgi-bin/mailman/listinfo/dev >>>> >
-----BEGIN PGP SIGNATURE-----
iQQcBAEBCgAGBQJV5vKJAAoJEKbUTLnGTOd7ND4f/AyPj/qKRUqsgcqeAxyUP3ex IXXmHpDMuaUJMjFHANeb/gfuBASmRMJwKVK6dttpBYlvyiEKFq9CaXMOMLz5EFjH xfOY+FnOyYTVx37dkE850dihGFWUu71+E9HnhBtE4qh0JEDWWQf0BXQkvph4egZP q0GDg1HRCrOCRG3VlOS0X0T+ORjUsmIM0zmvP/1Ox+9nVl5H2Ce9OLiPAI69DyzJ qxT0tMmm3c3PPtAyCwz/9uuM4uhFXASCRuMo+fOt8ptJN6gA3Oe/YkYgGMuQ6/wX LpkWXfU0x5bu4nyPltaevKLsQY03ewY7Ih4EqVG0fFecABfUskF7jAVYE1XeIdP2 OhGLtb4lJcRVZP6TQteg7esIwAT0KbJjta3s5rY9W+227Ps8VSh6W5sRBi51tys4 W1JqDDwlJUbT372pl8WAkEdDdqoBvxGhJbFbG+xAkOpAjbV0KCdR33TS4MqYrFkR SXwzntLq1qLBn96t+vGwRAxhTMTng8g87EfbZti362OvvbwUtSsVanwhgL+i/ubA 1eI3Vf/ERVjgvSDFS1R6vlHoiKuCLsWBCDy2VqdFEopV0SENo401SZGthmbbJyOg B0GGYXCwt76vUyPhorQZ7FMKdNovKqevfEXg78IehIXUN1k8QWQj2KBP2KDfTnC7 CDJMB23ib3irc2PRh+hxrruL4BXRnmrzQgm4o2eeP56IbYDYk7PuOyDy4B568QcQ e+R4QUstJTdWxcluxtdNHd4diPbPJw1P2DAw9NA4DkpKyIGDi1ut1RrIaEoulRjG mrTzPiobuV67RF1L1IOCffQoX2FOrVyepxhb6QskDVFir/w9qPZOr8yTPNAnzmR3 bNVBKg9L9GWKK4dbKE0FI68vqlwYJR8rik7Z++odkZhbHF+iKemO9/vXQnJVhaqu 9OYkj1EgnIsIK5rTvn2FmX2uXaRwTyRo/DUT1caV7o1t+O0vdQBlWd34zIceZJ0R DjVYEyZ9B6A/BsdxXmPz2gFPg+MeRyzC0QJN1naDWq7nO35ItD9ZaPJ1xvSNwHoT p0Cl1VbDRZ5HqjrpAHwPlaz8IQ9qzvG1nCc7b7ZHrxuV7nO18zpdJgRtpwujM2Sj irsriEs0eU6FebU9oVoZFVcFsGvxcAktxW62EzdHoc1TvjirCeXFPhg0sf1cAa7s 8yKWKQhX1gCucMrmGnXVfWuBg/zQkqBp8jUSjeSwcZXq7T+EJRwP+GjtTWRNHNHU 6p9aA6Ya6ZduRij5dks/6MDovs/FTfx4zwa4eEvTzQEFcTP7i2eOWmxoUhlfkFl3 qvBsJGI1qQsDX/g51Ydlldxs7IQf/e2ui52nbDB9uFNi5v9h9+umeaiMF6ZxSEs= =hFEn -----END PGP SIGNATURE-----