-
-
Notifications
You must be signed in to change notification settings - Fork 55
Fix parameter order in solve_model function calls in ifp_advanced #762
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
base: main
Are you sure you want to change the base?
Conversation
The solve_model function signature expects (ifp, c_init, a_init) and returns (c_out, a_out), but all call sites were passing parameters in the wrong order (a_init, c_init) and expecting returns in the wrong order (a_out, c_out). This fixes all 5 call sites in the lecture to use the correct parameter and return order: - Line 490: Fixed initial solve_model call - Line 497: Fixed timed solve_model call - Line 645: Fixed simulation section call - Line 737: Fixed return volatility loop call - Line 814: Fixed income volatility loop call Fixes #759
Investigation: Root Cause AnalysisI traced the bug back to its origin. The issue was introduced in commit 60235eb (PR #757) on December 2, 2025. What Changed in PR #757Before PR #757:
After PR #757:
The BugPR #757 refactored the function to use
However, none of the 5 call sites were updated to match the new signature, causing:
This PR fixes all 5 call sites to match the new function signature. |
Why PR #757's CI Didn't Catch the BugExcellent question! You're right that the cache would be invalidated when the code changed. Here's the actual reason the bug wasn't caught: The Critical Detail: Identical Initial ValuesThe bug didn't cause an execution error because all call sites initialize both parameters to the same value: Example 1 (lines 483-485): σ_init = jnp.empty((k, n))
for z in range(n):
σ_init = σ_init.at[:, z].set(ifp.s_grid)
a_init = σ_init.copy() # ← SAME VALUE!Example 2 (lines 642-644): a_init = s_grid[:, None] * jnp.ones(n_z)
c_init = a_init # ← SAME VALUE!Why This Masked the BugWhen you call: solve_model(ifp, a_init, c_init) # Wrong orderBut solve_model(ifp, c_init, a_init) # Correct orderThe function executed successfully, just with subtly incorrect iteration logic that wasn't obvious in the output. Why the Weekly Colab Check FailedThe weekly check may have:
The bug was silent but real - it executed without crashing but with incorrect computation logic. |
|
@jstac I still don't understand why But it appears some function calls weren't updated when we updated |
|
📖 Netlify Preview Ready! Preview URL: https://pr-762--sunny-cactus-210e3e.netlify.app (ca4439b) 📚 Changed Lecture Pages: ifp_advanced |
Summary
Fixes #759 - Weekly Colab execution check failure in the ifp_advanced lecture.
Problem
The
solve_modelfunction inlectures/ifp_advanced.mdhad a critical mismatch between its signature and how it was being called throughout the lecture:Function signature:
Issue: All 5 call sites were:
(ifp, a_init, c_init)instead of(ifp, c_init, a_init)a_vec, c_vec =instead ofc_vec, a_vec =This caused the notebook to fail during execution, as the wrong arrays were being used for asset grids vs consumption policies.
Changes
Fixed all 5 occurrences in
lectures/ifp_advanced.md:.block_until_ready()to useσ_starinstead ofa_star)Testing
This ensures the notebook will execute correctly when the Colab workflow runs again.
Closes #759