search

QuantEcon / ContinuousDPs.jl

Continuous state dynamic programming
BSD 3-Clause "New" or "Revised" License
13 stars 10 forks source link

Update notebooks #31

Closed oyamad closed 3 years ago

oyamad commented 4 years ago

Policy iteration for "Monetary Policy" is not working properly; see cell [49]: https://nbviewer.jupyter.org/github/QuantEcon/ContinuousDPs.jl/blob/5ec8906d73151cca630c66ffd8c9f06990e6578f/examples/cdp_ex_MF_jl.ipynb

amit1rrr commented 4 years ago

Sorry to intrude but I noticed this PR contains Jupyter notebook files that are hard to review on GitHub due to the JSON diffs.

You might want to checkout ReviewNB, code review tool for notebooks. Provides visual diffs & commenting for Jupyter notebooks on GitHub. Completely free for open source repositories.

Disclaimer: I built ReviewNB

diff_and_commenting

QBatista commented 3 years ago

@oyamad Which package versions are you using?

I run the notebook with Julia 1.0.3 and:

"ContinuousDPs" => v"0.1.0" "BasisMatrices" => v"0.6.0" "Plots" => v"0.29.9" "PyPlot" => v"2.9.0" "QuantEcon" => v"0.16.2"

With these versions, cell [49] also fails, but in a different way. Instead of cycling around 1e-5/1e-6, the algorithm gets stuck around 4580. There are small discrepancies in other cells, but none that are as large.

oyamad commented 3 years ago

@QBatista I rebased on master (with a change in Project.toml) and re-run the notebook. Policy iteration still fails in [49].

Julia v1.5.3 BasisMatrices v0.7.0 Plots v1.10.2 PyPlot v2.9.0 QuantEcon v0.16.2

codecov-io commented 3 years ago

Codecov Report

Merging #31 (3d5d1fe) into master (45ea759) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #31   +/-   ##
=======================================
  Coverage   87.97%   87.97%           
=======================================
  Files           2        2           
  Lines         158      158           
=======================================
  Hits          139      139           
  Misses         19       19

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 45ea759...3d5d1fe. Read the comment docs.

QBatista commented 3 years ago

@oyamad Cell [49] seems to work for me with a fresh install of Julia 1.5.3, the packages you mentioned, and IJulia v1.23.1. The output I obtained is: 

Compute iterate 50 with error 5.998727978415356e6
Compute iterate 100 with error 33359.630274752
Compute iterate 150 with error 85389.91195209554
Compute iterate 200 with error 209177.18297170423
Compute iterate 250 with error 8260.658798956169
Compute iterate 300 with error 153689.87928658456
Compute iterate 338 with error 4.661160346586257e-12
Converged in 338 steps

Do you know if this matches the output you used to obtain?

Also, could the issue come from a conflict with other packages that you have installed?

oyamad commented 3 years ago

An old version which worked is here: https://nbviewer.jupyter.org/github/QuantEcon/ContinuousDPs.jl/blob/f2720beb663b81783eb7e13ac9f22280ebb33c39/examples/cdp_ex_MF_jl.ipynb

See cell [48]. It converged in 9 steps.

QBatista commented 3 years ago

I have tried to run the notebook with different version of Optim, BasisMatrices, and Julia. The results are summarized below:

Optim v0.17.2 -> Fails Optim v0.17.0 -> Fails Optim v0.16.0 -> Fails Optim v0.16.0, BasisMatrices v0.6.0 -> Fails Julia 1.0.3, Optim v0.16.0, BasisMatrices v0.6.0 -> Fails Julia 0.7.0, Optim v0.16.0, BasisMatrices v0.6.0 -> Fails Binder -> Fails

With the latest versions of Julia and other packages, I have also tried changing the parametrization of the spline basis.

m = [20, 20] -> 6 iterations but strange looking policy surface m = [15, 15] -> VFI diverges, strange surface with PFI m = [12, 12] -> VFI gets stuck m = [18, 18] -> VFI diverges m = [9, 9] -> Seems fine, 24 iterations with PFI

k = [2, 2] -> Seems fine, 8 iterations

This suggests some numerical instability with the default order (k = [3, 3]). Could you try the last case (k = [2, 2]) on your computer? It seems to be a good solution for this problem.

QBatista commented 3 years ago

@oyamad The original Matlab program from CompEcon solves the problem in 4 iterations with PFI (called "newton" in their code). I think the difference is coming from the choice of initial basis coefficients. They first compute an LQ approximation to the value function and use it to compute initial basis coefficients. If I use the same initial coefficients with the code in ContinuousDPs.jl, it also takes 4 iterations to get convergence. The value of the convergence criterion is slightly different but the difference seems to be minor (1.8e-12 vs. 1.6e-12).

oyamad commented 3 years ago

@QBatista Great! So it is related to PR #26, which offers an option to provide an initial condition.

oyamad commented 3 years ago

Superceded by #38.