Skip to content

[wealth_dynamics.md] Update np.random → Generator API - #989

Merged
jstac merged 1 commit into
mainfrom
update-rng-wealth-dynamics
Jul 21, 2026
Merged

[wealth_dynamics.md] Update np.random → Generator API#989
jstac merged 1 commit into
mainfrom
update-rng-wealth-dynamics

Conversation

@Chihiro2000GitHub

@Chihiro2000GitHub Chihiro2000GitHub commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR migrates legacy NumPy random API usage in wealth_dynamics.md as part of QuantEcon/meta#299.

The migration is limited to the ordinary-Python code cells. The code around update_cross_section is intentionally left unchanged — see Details.

Related PRs and issues

Before opening this PR I surveyed open PRs and issues touching this lecture:

  • Open PRs: no other open PR touches wealth_dynamics.md.
  • Already merged into main (reflected in this branch): #574, #581.
  • Closed without merging (earlier style-guide / JAX-conversion attempts): #513, #626, #628, #630, #632, #634, #636, #638, #639.
  • Related open issue: #970 — random seed in the parallel update_cross_section code; deliberately left for a follow-up (see Details).
  • An unrelated issue that still touches this lecture: #512 — a title-formatting tracking issue.

Details

In the plain Python cells, np.random.* calls are replaced with an explicit rng = np.random.default_rng(). rng is defined at its first use in the main text and reused afterwards; the wd_ex1 solution block defines its own rng to stay self-contained. No fixed seed is introduced.

Code around update_cross_section left unchanged (deliberate). While reviewing this file I found the pre-existing issue #970 ("Add random seed to wealth dynamics lecture"). It concerns update_cross_section, which is @jit(parallel=True) and draws unseeded np.random.randn() inside a prange loop, causing the Gini-vs-μ_r figure to change on every rebuild. Since that discussion is still open, I've deliberately left that code — and the related code that depends on it — untouched in this PR:

  • update_cross_section draws inside a prange loop, where Numba's Generator support is not thread-safe.
  • update_states is used by both the parallel update_cross_section and the non-parallel wealth_time_series, so it cannot be migrated on its own.
  • Because of this, migrating only the initial draw in wealth_time_series (line ~368) would leave the per-period shocks (drawn via update_states) on the legacy global np.random state, mixing two RNG sources within a single simulation — inconsistent, and it would not actually remove the hidden global state from that code path.

If you'd like, I'm happy to handle this update_cross_section code (RNG migration and/or the #970 reproducibility fix) in a separate follow-up — just let me know how you'd prefer to proceed.

Hi @mmcky and @HumphreyYang, I'd be grateful if you could take a look when you have time.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rs2tDEcDXLnfMnM88v8e1D
@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-989--sunny-cactus-210e3e.netlify.app

Commit: d0b5b67

📚 Changed Lectures


Build Info

@jstac

jstac commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Thanks @Chihiro2000GitHub . This looks good.

Can you please confirm that no earlier PRs also touch this lecture.

@Chihiro2000GitHub

Copy link
Copy Markdown
Contributor Author

Thanks @jstac!

Confirmed — no other PRs touch this lecture. The earlier PRs that touched it are either already merged into main (#574, #581, both reflected in this branch) or were closed without merging (#513, #626, #628, #630, #632, #634, #636, #638, #639 — earlier style-guide / JAX-conversion attempts).

I also re-checked open issues: the only one relevant to this migration is #970 (random seed in the parallel update_cross_section code), which I noted in the Details.

#512 also touches this lecture, but it's just a title-formatting issue, unrelated to this RNG migration.

For context, I did survey open PRs and issues when I created this PR, but the PR body didn't make that survey explicit — I've now lightly edited it to state it clearly.

@jstac

jstac commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Hi @Chihiro2000GitHub , thanks for the update.

I'm a bit confused. Do these statements contradict each other?

Confirmed — no other PRs touch this lecture.

#512 also touches this lecture

@Chihiro2000GitHub

Copy link
Copy Markdown
Contributor Author

Hi @jstac

Sorry, my wording was not clear. There is no contradiction — I was talking about two different things:

・No other PR touches this lecture.
#512 is an issue, not a PR. It only lists this lecture in a checklist for a title-formatting fix, so it is not related to this RNG migration.

So there is no open PR that overlaps with this one.

@jstac

jstac commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Thanks @Chihiro2000GitHub . I should have looked at #512 myself.

In this context 'touching' refers to changing, at least for our team, but the English is unusual and probably confusing.

This PR looks good to me. It's good that you left the parallel code. Merging.

@jstac
jstac merged commit fccf5a9 into main Jul 21, 2026
1 check passed
@jstac
jstac deleted the update-rng-wealth-dynamics branch July 21, 2026 23:02
@Chihiro2000GitHub

Copy link
Copy Markdown
Contributor Author

Thanks @jstac!

I understand now — since an issue does not change anything, saying an issue "touches" the lecture is confusing.

I see what you mean now, and I will keep it in mind going forward.

Thanks for the review and for merging!

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.

2 participants