Skip to content

FIX: two minor modifications in lqcontrol#498

Merged
mmcky merged 2 commits intoQuantEcon:masterfrom
shizejin:fix_lqcontrol
Aug 7, 2019
Merged

FIX: two minor modifications in lqcontrol#498
mmcky merged 2 commits intoQuantEcon:masterfrom
shizejin:fix_lqcontrol

Conversation

@shizejin
Copy link
Copy Markdown
Member

I made two fixes for LQ and LQMarkov classes.

  1. There was a typo in calculating the values of the last period states for LQMarkov models. I forgot to left multiply the C matrix to the vector of shocks. I have fixed this and modified tests correspondingly.

  2. The code was computing stationary values each time we call compute_series, which is unnecessary since the optimal policy F are stored as class attribute and do not change. This increases running time especially for lectures that simulate multiple paths for the same model, e.g. "How to Pay for a War: Part 1". I modified it to only compute stationary values once when compute_series method is called for the first time. By doing so, the running time of "How to Pay for a War: Part 1" decreases from 42s to 12s.

The first point is closely related to the work about the series of Markov Jump LQ DP lectures. I feel sorry that I fail to spot this typo. It would be great if we can make another release before we publish those lectures. However, since this bug only matters for values in the last period and almost has no influence on the patterns of the graphs that we draw, I guess it will also be fine that we publish lectures first and include these fixes in a release with other commits later.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 94.097% when pulling cfd14dc on shizejin:fix_lqcontrol into 15b3b93 on QuantEcon:master.

@jstac
Copy link
Copy Markdown
Contributor

jstac commented Jul 19, 2019

Thanks @shizejin, I appreciate the fix.

@oyamad @mmcky This looks good to me.

@mmcky mmcky merged commit 7ea7b18 into QuantEcon:master Aug 7, 2019
@mmcky mmcky mentioned this pull request Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants