[ifp_egm] update namedtuple #130

shlff · 2023-10-16T01:31:39Z

This PR fix #128 .

netlify · 2023-10-16T01:31:43Z

✅ Deploy Preview for incomparable-parfait-2417f8 ready!

Name	Link
🔨 Latest commit	`32a8f9b`
🔍 Latest deploy log	https://app.netlify.com/sites/incomparable-parfait-2417f8/deploys/65e531f94fddfe0008437304
😎 Deploy Preview	https://deploy-preview-130--incomparable-parfait-2417f8.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2023-10-16T01:45:30Z

🚀 Deployed on https://65e53524b6933606366d8586--incomparable-parfait-2417f8.netlify.app

jstac · 2024-03-03T20:48:00Z

@shlff , this is labeled as "in-work". Is it ready to review?

shlff · 2024-03-04T00:15:10Z

Thanks @jstac . I've been reviewing this PR, and I will get back to you very soon.

shlff · 2024-03-04T02:46:32Z

Thanks @jstac . This PR is ready to review as well, and please find a preview:

https://65e53524b6933606366d8586--incomparable-parfait-2417f8.netlify.app/ifp_egm

In addition to using namedtuple, I also tidied up some code styles and words.

HumphreyYang · 2024-03-04T03:51:28Z

Many thanks @shlff, @jstac, and @mmcky. I will review PRs in this repo tonight.

HumphreyYang

Many thanks @shlff. Please find minor code improvement proposals below:

HumphreyYang · 2024-03-04T11:02:26Z

lectures/.ipynb_checkpoints/ifp_egm-checkpoint.md

@@ -0,0 +1,619 @@
+---


Please remove this file as it is a checkpoint file.

HumphreyYang · 2024-03-04T11:03:15Z

lectures/ifp_egm.md

+Household = namedtuple('Household', ('β', 'R', 'γ', 's_size', 'y_size', \
+                                     's_grid', 'y_grid', 'P'))
+
+def create_household(R=1.01,             # gross interest rate
+                     β=0.99,             # discount factor
+                     γ=1.5,              # CRRA preference parameter
+                     s_max=16,           # savings grid max
+                     s_size=200,         # savings grid size
+                     ρ=0.99,             # income persistence
+                     ν=0.02,             # income volatility
+                     y_size=25):         # income grid size


Let's migrate the comments to where the namedtuple is defined : )

I agree --- in fact let's have both, because the arguments are not identical to the elements of the namedtuple. Some overlap is fine.

HumphreyYang · 2024-03-04T11:04:21Z

lectures/ifp_egm.md

+    return Household(β=β, R=R, γ=γ, s_size=s_size, y_size=y_size, \
+               s_grid=s_grid, y_grid=y_grid, P=P)


Suggested change

return Household(β=β, R=R, γ=γ, s_size=s_size, y_size=y_size, \

s_grid=s_grid, y_grid=y_grid, P=P)

return Household(β=β, R=R, γ=γ, s_size=s_size, y_size=y_size,

s_grid=s_grid, y_grid=y_grid, P=P)

Nice catch @HumphreyYang . Not needed after comma.

HumphreyYang · 2024-03-04T11:23:41Z

lectures/ifp_egm.md

-    constants, sizes, arrays = model
+    β, R, γ, s_size, y_size, s_grid, y_grid, P = model

-    β, R, γ = constants
-    s_size, y_size = sizes
-    s_grid, y_grid, P = arrays
+    constants = β, R, γ
+    sizes = s_size, y_size
+    arrays = s_grid, y_grid, P


This is a small observation on the original code. Here we are packing constants, sizes, and arrays for the function K_egm, but later in K_egm, we unpack them again:

def K_egm(a_in, σ_in, constants, sizes, arrays): """ The vectorized operator K using EGM. """ # Unpack β, R, γ = constants s_size, y_size = sizes s_grid, y_grid, P = arrays

perhaps we can remove the packing and pass the model directly into K_egm to remove this redundancy.

Thanks @HumphreyYang . This is a good idea. I have thought and tried about this while coding but didn't find an elegant way to do then.

HumphreyYang · 2024-03-04T11:26:27Z

lectures/ifp_egm.md

-model = ifp()
-constants, sizes, arrays = model
-β, R, γ = constants
-s_size, y_size = sizes
+model = create_household()
+β, R, γ, s_size, y_size, s_grid, y_grid, P = model
+arrays = s_grid, y_grid, P
 s_grid, y_grid, P = (np.array(a) for a in arrays)

 @jitclass(ifp_data)


Instead of creating global variables in the cell below, I propose that we define the default values for class IFP using model directly:

ifp_data = [ ('R', float64), ('β', float64), ('γ', float64), ('P', float64[:, :]), ('y_grid', float64[:]), ('s_grid', float64[:]) ] # Use the JAX IFP data as our defaults for the Numba version model = create_household() β, R, γ, s_size, y_size, s_grid, y_grid, P = model arrays = s_grid, y_grid, P s_grid, y_grid, P = (np.array(a) for a in arrays) @jitclass(ifp_data) class IFP: def __init__(self, R=R, β=β, γ=γ, P=np.array(P), y_grid=np.array(y_grid), s_grid=s_grid): self.R, self.β, self.γ = R, β, γ self.P, self.y_grid = P, y_grid self.s_grid = s_grid # Recall that we need R β < 1 for convergence. assert self.R * self.β < 1, "Stability condition violated." def u_prime(self, c): return c**(-self.γ) def u_prime_inv(self, u_prime): return u_prime**(-1/self.γ)

->

ifp_data = [ ('R', float64), ('β', float64), ('γ', float64), ('y_size', int64), ('s_size', int64), ('P', float64[:, :]), ('y_grid', float64[:]), ('s_grid', float64[:]), ] # Use the JAX IFP data as our defaults for the Numba version model = create_household() @jitclass(ifp_data) class IFP: def __init__(self, R=model.R, β=model.β, γ=model.γ, P=np.array(model.P), y_grid=np.array(model.y_grid), s_grid=np.array(model.s_grid)): self.R, self.β, self.γ = R, β, γ self.P, self.y_grid = P, y_grid self.s_grid = s_grid self.y_size, self.s_size = len(y_grid), len(s_grid) # Recall that we need R β < 1 for convergence. assert self.R * self.β < 1, "Stability condition violated." def u_prime(self, c): return c**(-self.γ) def u_prime_inv(self, u_prime): return u_prime**(-1/self.γ)

Note that y_size will be removed from the global variable after this change. We will need to change the function successive_approx_numba below that is dependent on global y_size.

Also note that we will need to add int64 in the import:

from numba import njit, float64, int64

Hi @jstac, please let us know if you think it is a good improvement to the current version when you are available.

I agree @HumphreyYang, this is neater.

jstac · 2024-03-04T18:20:35Z

lectures/ifp_egm.md

    # Shift to JAX arrays
    P, y_grid = jax.device_put((P, y_grid))

    s_grid = jnp.linspace(0, s_max, s_size)
-    sizes = s_size, y_size
    s_grid, y_grid, P = jax.device_put((s_grid, y_grid, P))


s_grid and y_grid are already on the device.

jstac · 2024-03-04T18:43:26Z

Many thanks for your input @shlff and @HumphreyYang . I'll tidy this up and merge it. Please leave it to me.

jstac · 2024-03-04T19:01:13Z

Hey @shlff , I just looked back at the original issue #128 and the aim is to get rid of the class.

The only class is for the numba version, which is not fixed in this PR. Let's be careful when we say the PR fixes an issue, since we don't want to close an issue that's not addressed.

As background, I believe we should move away from using classes whenever we jit-compile with either numba or JAX. The reasons are:

functions are easier to debug -- we can write each one in pure Python and then incrementally add @jit to each one and debug the optimization
If we use classes, then it's always hard to delineate what should be a method and what should be a function acting on the class. This leads to inconsistency.
Classes are a little dangerous -- if we create an instance foo and then change parameter foo.a, it will not change foo.b where b is defined internally by self.b = 2 * a. With a namedtuple, we are forced to create a new instance (which is usually cheap).

CC @mmcky @thomassargent30

jstac · 2024-03-04T19:10:37Z

@mmcky This PR includes the ipynb checkpoint. I've closed the PR but I wonder if there's an easy way to unify the .gitignore files across the repos. If not, would you mind to do a quick review of the .gitignore files and check they are roughly the same?

shlff · 2024-03-04T20:46:47Z

Thanks @HumphreyYang and @jstac . I've read your comments and those are valuable!

mmcky · 2024-03-04T22:29:19Z

@mmcky This PR includes the ipynb checkpoint. I've closed the PR but I wonder if there's an easy way to unify the .gitignore files across the repos. If not, would you mind to do a quick review of the .gitignore files and check they are roughly the same?

I have tried a couple times to see if we could use quantecon/.github as a place to store default files for repositories. It haven't gotten that to work effectively to date. I will open an issue and see if there have been improvements on GitHub to support this.

In the meantime I will do a quick review of them. My feeling is lecture-jax is an outlier. I have come across a few issues recently with some of the syntax used that I am working to fix up.

Update ifp_egm.md

5b3941e

shlff added the in-work label Oct 16, 2023

github-actions bot temporarily deployed to pull request October 16, 2023 01:45 Inactive

tidyup

32a8f9b

github-actions bot temporarily deployed to pull request March 4, 2024 02:42 Inactive

shlff added ready and removed in-work labels Mar 4, 2024

mmcky requested review from jstac and HumphreyYang March 4, 2024 03:48

HumphreyYang reviewed Mar 4, 2024

View reviewed changes

jstac reviewed Mar 4, 2024

View reviewed changes

jstac closed this Mar 4, 2024

jstac mentioned this pull request Mar 4, 2024

[ifp_egm] Tidy up EGM, remove jitclass #144

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ifp_egm] update namedtuple #130

[ifp_egm] update namedtuple #130

shlff commented Oct 16, 2023

netlify bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

jstac commented Mar 3, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

HumphreyYang commented Mar 4, 2024

HumphreyYang left a comment

HumphreyYang Mar 4, 2024

HumphreyYang Mar 4, 2024 •

edited

Loading

jstac Mar 4, 2024

HumphreyYang Mar 4, 2024

jstac Mar 4, 2024

HumphreyYang Mar 4, 2024

shlff Mar 4, 2024

HumphreyYang Mar 4, 2024 •

edited

Loading

jstac Mar 4, 2024

jstac Mar 4, 2024

jstac commented Mar 4, 2024

jstac commented Mar 4, 2024

jstac commented Mar 4, 2024

shlff commented Mar 4, 2024

mmcky commented Mar 4, 2024

		return Household(β=β, R=R, γ=γ, s_size=s_size, y_size=y_size, \
		s_grid=s_grid, y_grid=y_grid, P=P)

[ifp_egm] update namedtuple #130

[ifp_egm] update namedtuple #130

Conversation

shlff commented Oct 16, 2023

netlify bot commented Oct 16, 2023 • edited Loading

✅ Deploy Preview for incomparable-parfait-2417f8 ready!

github-actions bot commented Oct 16, 2023 • edited Loading

jstac commented Mar 3, 2024

shlff commented Mar 4, 2024

shlff commented Mar 4, 2024

HumphreyYang commented Mar 4, 2024

HumphreyYang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HumphreyYang Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jstac commented Mar 4, 2024

jstac commented Mar 4, 2024

jstac commented Mar 4, 2024

shlff commented Mar 4, 2024

mmcky commented Mar 4, 2024

netlify bot commented Oct 16, 2023 •

edited

Loading

github-actions bot commented Oct 16, 2023 •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading

HumphreyYang Mar 4, 2024 •

edited

Loading