Skip to content
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

[ak2] Update to Transitions in an Overlapping Generations Model lecture #508

Closed
wants to merge 1 commit into from

Conversation

Jiarui-ZH
Copy link
Contributor

Update to Transitions in an Overlapping Generations Model lecture basing on #491

Content

  • Just before equation 31.3 add brackets explained that it is taking the first-order condition over the previous equation
  • Before equation 31.8, Added a Wikipedia link or explaination on the Lagrangie condition

Code

  • Removed the usage of njit

Copy link

netlify bot commented Jul 8, 2024

Deploy Preview for taupe-gaufre-c4e660 ready!

Name Link
🔨 Latest commit 8d4597f
🔍 Latest deploy log https://app.netlify.com/sites/taupe-gaufre-c4e660/deploys/66b39b16b8477f0007016d93
😎 Deploy Preview https://deploy-preview-508--taupe-gaufre-c4e660.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@mmcky
Copy link
Contributor

mmcky commented Jul 8, 2024

@Jiarui-ZH the execution error is in this code cell

------------------
W, r_next, τ, τ_next = W_hat, r_hat, τ_hat, τ_hat
δy, δo_next = 0, 0

Cy_opt, U_opt, _ = brent_max(Cy_val,            # maximand
                             1e-6,              # lower bound
                             W*(1-τ)-δy-1e-6,   # upper bound
                             args=(W, r_next, τ, τ_next, δy, δo_next, β))

Cy_opt, U_opt
------------------

with the following error


Failed in nopython mode pipeline (step: nopython frontend)

This error may have been caused by the following argument(s):

  • argument 0: �[1mCannot determine Numba type of <class 'function'>�[0m

@mmcky
Copy link
Contributor

mmcky commented Jul 23, 2024

@Jiarui-ZH I see you have removed njit should these be changed to jit rather than deleted?

@Jiarui-ZH
Copy link
Contributor Author

@mmcky I ran a test and found out there's almost no runtime difference between having and not having jit. I have spoken to John and he suggested to just remove it all together.

@mmcky
Copy link
Contributor

mmcky commented Jul 23, 2024

@mmcky I ran a test and found out there's almost no runtime difference between having and not having jit. I have spoken to John and he suggested to just remove it all together.

Thanks @Jiarui-ZH. Are these functions used later in any jitted functions?

@mmcky
Copy link
Contributor

mmcky commented Aug 7, 2024

@Jiarui-ZH there is still an execution error being reported to do with numba jit.
Can you please test this on your end and review.

@shizejin
Copy link
Member

shizejin commented Aug 7, 2024

The quantecon.brent_max can only take njit function as an input, that's why there is an execution error when we do not njit the maximand function Cy_val.

If we are going to remove all the njit, we should perhaps use some minimizer from scipy.optimize to replace brent_max.

I am not sure about the decline in speed with such modification though, because calling minimizer from scipy seems costly to me. But I would agree to removing all the njit if there is no sacrifice of speed :)

@Jiarui-ZH
Copy link
Contributor Author

Jiarui-ZH commented Aug 7, 2024

I had to revert back to the first commit because I stil had trouble resolving the problem and the codes got messy, I will keep working on this

@mmcky
Copy link
Contributor

mmcky commented Aug 8, 2024

thanks @shizejin for your input here.

@Jiarui-ZH I suspect the jit will be beneficial in this lecture.

I propose we close this PR given there aren't a lot of editorial changes. Then you can open a PR on the new repo once we move this lecture. Is that OK @Jiarui-ZH?

@shizejin
Copy link
Member

@mmcky @Jiarui-ZH Thanks a lot for the great dicussion and contribution for improving ak2. I am just checking in if there is anything I could help with this PR? :)

@mmcky
Copy link
Contributor

mmcky commented Aug 16, 2024

thanks @shizejin -- I am going to close this PR now so we can migrate to the new lecture series.

Then it would be great to get your input into this lecture. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants