Skip to content

[mccall_model_with_separation] Conversion to JAX - #501

Merged
HumphreyYang merged 3 commits into
mainfrom
mcim
Jul 30, 2025
Merged

[mccall_model_with_separation] Conversion to JAX#501
HumphreyYang merged 3 commits into
mainfrom
mcim

Conversation

@jstac

@jstac jstac commented Jul 29, 2025

Copy link
Copy Markdown
Contributor

Changing job search with separation to use JAX

@longye-tian @HumphreyYang

Could one of you please review this? The last figure looks a bit different.

@github-actions

github-actions Bot commented Jul 29, 2025

Copy link
Copy Markdown

@github-actions
github-actions Bot temporarily deployed to pull request July 29, 2025 08:44 Inactive
@github-actions
github-actions Bot temporarily deployed to pull request July 29, 2025 08:46 Inactive
@longye-tian

Copy link
Copy Markdown
Contributor

Hi John, @jstac ,

I just compare the numba version and the jax version.

I find that Jax version is correct, and numba version has a small problem.

In the original numba code, we didn't put everything in functions, so we have in the first loop over unemployment compensation c from 2 to 12.

So the model in the numba version when looping over discount factor and separation rate, we have c=12 instead of the default value of 6.

Similar happens when we loop discount factor openly without the function, but since the last loop is 0.98 over 0.99 the difference is not too big.

So the conclusion is that the Jax version is correct and better.

Best,
Longye

@longye-tian longye-tian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Jax version is correct

w_bar = compute_reservation_wage(model)
return w_bar

for i, c in enumerate(c_vals):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we didn't put this part in the function.

So later one when we loop over discount factor, we have c=12 which is the value from the last loop

w_bar = compute_reservation_wage(model)
return w_bar

for i, β in enumerate(beta_vals):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar in here

w_bar = compute_reservation_wage(model)
return w_bar

for i, α in enumerate(alpha_vals):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

similar in here

@HumphreyYang

HumphreyYang commented Jul 30, 2025

Copy link
Copy Markdown
Member

Many thanks @jstac and @longye-tian,

I agree. One way to verify that this is the issue is to add mcm = McCallModel() before we run the experiment at each cell so it refresh the parameter that is carried over from the previous experiment.

I got the same solution as JAX once we do that:
https://gist.github.com/a8b53da3d0a4fbc2956bc29e1d000013.git

@mmcky

mmcky commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

@longye-tian and @HumphreyYang outstanding work.

  • open a companion PR to fix numba implementation.

@HumphreyYang

Copy link
Copy Markdown
Member

Thanks @mmcky,

Would you like me to open a PR that fix the numba code? Just curious if we are replacing the numba code with JAX code.

@mmcky

mmcky commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Thanks @mmcky,

Would you like me to open a PR that fix the numba code? Just curious if we are replacing the numba code with JAX code.

Ah. Thank you @HumphreyYang. I am rushing around the repos at the moment. You're 100% correct. This is replacing the numba code (I thought from the discussion we had both versions). Nothing to do here :-)

@mmcky

mmcky commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Thanks @HumphreyYang and @longye-tian for your helpful reviews.

@jstac just checking do you intend to make any other changes before merge?

@jstac

jstac commented Jul 30, 2025

Copy link
Copy Markdown
Contributor Author

Many thanks @longye-tian for the careful review, it's much appreciated. Thanks also @HumphreyYang for confirming and @mmcky for supervising.

As @HumphreyYang says, we only need the JAX version. Can someone please do one more overall sanity check and make sure everything makes sense. Then please flag it for merging. I don't need to see it again.

@HumphreyYang

HumphreyYang commented Jul 30, 2025

Copy link
Copy Markdown
Member

Many thanks @jstac and @mmcky,

I will update the static figures in the lecture to the new version and change a few variable names to unicode. Other than those, it looks good to me!

@github-actions
github-actions Bot temporarily deployed to pull request July 30, 2025 12:05 Inactive
@github-actions
github-actions Bot temporarily deployed to pull request July 30, 2025 12:07 Inactive
@HumphreyYang HumphreyYang changed the title job search update [mccall_model_with_separation] Conversion to JAX Jul 30, 2025
@HumphreyYang
HumphreyYang merged commit a48f019 into main Jul 30, 2025
7 checks passed
@HumphreyYang
HumphreyYang deleted the mcim branch July 30, 2025 22:51
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