Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify tauchen and rouwenhorst API #664

Merged
merged 8 commits into from
Dec 17, 2022
Merged

Conversation

Smit-create
Copy link
Member

Fixes #363
#663

@coveralls
Copy link

coveralls commented Dec 6, 2022

Coverage Status

Coverage decreased (-0.04%) to 95.275% when pulling 4bef50a on Smit-create:i-663 into 67f391e on QuantEcon:master.

@oyamad
Copy link
Member

oyamad commented Dec 6, 2022

Maybe add a note after this?

.. math::
\theta_2 =
\begin{bmatrix}
p & 1 - p \\
1 - q & q \\
\end{bmatrix}
.. math::
\theta_{n+1} =
p
\begin{bmatrix}
\theta_n & 0 \\
0 & 0 \\
\end{bmatrix}
+ (1 - p)
\begin{bmatrix}
0 & \theta_n \\
0 & 0 \\
\end{bmatrix}
+ q
\begin{bmatrix}
0 & 0 \\
\theta_n & 0 \\
\end{bmatrix}
+ (1 - q)
\begin{bmatrix}
0 & 0 \\
0 & \theta_n \\
\end{bmatrix}

Something like "where p = q = (1 + rho) / 2."

Copy link
Member

@oyamad oyamad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me!

@jstac
Copy link
Contributor

jstac commented Dec 7, 2022

Nice work @Smit-create !

This looks good to me too but please coordinate with @mmcky to make sure the lectures are suitably modified.

@Smit-create
Copy link
Member Author

Thanks for the review. Once this is merged, I will quickly start fixing the lecture codes.

@jstac
Copy link
Contributor

jstac commented Dec 7, 2022

Thanks @Smit-create , that would be great. As you know, there is also https://github.com/QuantEcon/book-dp1/tree/main/source_code_py

It would be great if you could also help modify https://github.com/QuantEcon/cbc_workshops so the code there continues to work.

@Smit-create
Copy link
Member Author

@jstac Sure, I'll fix that too.

@mmcky
Copy link
Contributor

mmcky commented Dec 7, 2022

Thanks for the review. Once this is merged, I will quickly start fixing the lecture codes.

@Smit-create the best way to go here is to install this branch version on a PR marked do not merge and you can use that for testing. You just need to update the environment.yml file to use the branch version using git+https. I will try and set that up for you later this evening if not sure of the syntax.

@@ -12,11 +12,11 @@
from numba import njit


def rouwenhorst(n, ybar, sigma, rho):
def rouwenhorst(n, rho, sigma, mu=0.):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smit-create while it is difficult in this change to preserve backward compatibility, can you add a deprecation notice to the doctoring at the bottom.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

@mmcky
Copy link
Contributor

mmcky commented Dec 17, 2022

@Smit-create I will merge this into master but we will need lecture PR's tested prior to releasing v0.6.0

@mmcky mmcky merged commit 0e21854 into QuantEcon:master Dec 17, 2022
@Smit-create Smit-create deleted the i-663 branch December 17, 2022 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify and improve tauchen and rouwenhorst
5 participants