Skip to content

[newton_method] Update lecture code and fix typos #564

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kp992
Copy link
Contributor

@kp992 kp992 commented Aug 19, 2025

This PR:

  • updates the code to latest style guide like using typing.NamedTuple
  • pep8 fixes
  • typos and math errors
  • converts code to use jax

@kp992 kp992 requested review from mmcky and HumphreyYang August 19, 2025 03:23
@kp992 kp992 added the ready label Aug 19, 2025
Copy link

github-actions bot commented Aug 19, 2025

@github-actions github-actions bot temporarily deployed to pull request August 19, 2025 03:49 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 19, 2025 03:49 Inactive
Copy link
Member

@HumphreyYang HumphreyYang left a comment

Choose a reason for hiding this comment

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

Many thanks, @kp992! Great changes!

In this lecture, we used autograd because we were trying to avoid using JAX in the intermediate series. Since we are now migrating to JAX, I think we can use jax.jacobian here by adapting some functions from this lecture:

https://jax.quantecon.org/newtons_method.html

Let me know if that sounds good to you!

(CC @mmcky)

@kp992
Copy link
Contributor Author

kp992 commented Aug 20, 2025

Thanks @HumphreyYang for the review. I was wondering if we want to keep just a single lecture on Newton's method using JAX or if we are planning to keep two versions -- one here and the other in JAX series?

@HumphreyYang
Copy link
Member

Hi @kp992,

Thanks @HumphreyYang for the review. I was wondering if we want to keep just a single lecture on Newton's method using JAX or if we are planning to keep two versions -- one here and the other in JAX series?

Yes! Our plan is to replace them with JAX implementation and keep the old JAX series and rename it to 'GPU computing for computational economics'.

@mmcky mmcky added lecture and removed content labels Aug 21, 2025
@kp992
Copy link
Contributor Author

kp992 commented Aug 21, 2025

Thanks @HumphreyYang for the clarification. I will update the lecture to use JAX then.

@kp992 kp992 removed the ready label Aug 21, 2025
@kp992 kp992 marked this pull request as draft August 21, 2025 01:00
@kp992
Copy link
Contributor Author

kp992 commented Aug 21, 2025

Our plan is to replace them with JAX implementation and keep the old JAX series

So basically, we should host the same lecture in this series too, right? I mean we can copy over the JAX lecture here and update the code with styling fixes and typos?

@HumphreyYang
Copy link
Member

HumphreyYang commented Aug 21, 2025

Our plan is to replace them with JAX implementation and keep the old JAX series

So basically, we should host the same lecture in this series too, right? I mean we can copy over the JAX lecture here and update the code with styling fixes and typos?

Hi @kp992, yes, but I recall that for some lectures, the JAX version we wrote is a simplified version without discussions. For code that has overlaps, I think we can copy things over (like the newton)

@kp992 kp992 marked this pull request as ready for review August 22, 2025 01:37
@kp992
Copy link
Contributor Author

kp992 commented Aug 22, 2025

The lecture is now updated to optimized version of JAX and removed numpy/autograd support.

@github-actions github-actions bot temporarily deployed to pull request August 22, 2025 02:02 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 22, 2025 02:03 Inactive
@mmcky mmcky requested a review from Copilot August 22, 2025 04:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the Newton's method lecture by updating the code to follow current best practices, fixing typos, and correcting mathematical notation errors. The changes improve code quality and educational clarity while maintaining the lecture's pedagogical structure.

Key changes:

  • Migrates from deprecated autograd to jax for automatic differentiation
  • Updates from collections.namedtuple to typing.NamedTuple for better type support
  • Corrects mathematical notation and fixes various typos throughout

```

```{code-cell} ipython3
k_star_approx_newton
```

The result confirms the descent we saw in the graphs above: a very accurate result is reached with only 5 iterations.
The result confirms the convergence we saw in the graphs above: a very accurate result is reached with only 5 iterations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The result confirms the convergence we saw in the graphs above: a very accurate result is reached with only 5 iterations.
The result confirms convergence we saw in the graphs above: a very accurate result is reached with only 5 iterations.

Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

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

nice work @kp992 -- I have left some minor comments for you. Mainly just minor adjustments for pep8 etc.

@kp992
Copy link
Contributor Author

kp992 commented Aug 23, 2025

Thanks for the review @mmcky.

@github-actions github-actions bot temporarily deployed to pull request August 23, 2025 04:53 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 23, 2025 04:53 Inactive
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.

3 participants