-
-
Notifications
You must be signed in to change notification settings - Fork 205
Modified this apparently on flight home from CEF Singapore #1505
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: master
Are you sure you want to change the base?
Conversation
|
@wdu9 do you want to make your particular edits in response to the @CDC2WXD comments in SSJ_explanation.ipynb, and then I can review? @llorracc, I did a pretty extensive tidy up of KS-HARK-Presentation.ipynb after you had done these edits. I suggest keeping the KS-HARK-Presentation.ipynb I edited. Let's quickly discuss in person next Zoom. |
|
Scratch that --- I will address the comments including the ones to @wdu9. |
|
I think this PR is going to be harder to resolve. It looks like it's from even earlier than the prior one, and the files have been changed substantially since then. The "new" file in this PR might actually be an old version of a different file, under an old name. I guess Akshay will tackle this in a fresh branch? Funny enough, CDC's primary comment from June 2024 was exactly what I wrote in my review on the other PR (unaware Chris wrote it here): the model needs to be explained in math and/or words. |
|
Thanks @mnwhite Do you have a complete inventory of the HANK related notebooks and material in HARK? If so, is that inventory described somewhere that would be easy to find for someone looking for precisely this material? If not, I think it would be very useful to have such an inventory. |
|
The SSJ and HANK-adjacent notebooks were moved from /examples/SSJ-example/ to /examples/ConsNewKeynesianModel/ in the master branch. There are currently five notebooks there:
I think KrusellSmith_solved_by_HANK.ipynb in this branch is a very old/preliminary version of what is now HANKFiscal_example.ipynb, but don't hold me to that. |
This is a recreation of the substantive changes from #1505 . Requested / suggested changes that CDC left as comments were not made.
|
Substantive changes have been incorporated via #1568 |
|
Re-opened because Akshay wants it left as to be completed. |
|
Can this PR be archived now in some way? My understanding is that the remaining work on this is to address some comments from CDC that require new writing. Could those comments be moved to an issue so that we can close out a PR that will never be merged because it's been superceded? |
|
@mnwhite, I'm OK if you want to turn them into an issue. You should be able to compare he commit before this one to this commit to see what I changed/edited. |
@wdu9, @akshayshanker , could you both review?
@wdu9, there are some html comments embedded for you. Search CDC.
Please ensure your pull request adheres to the following guidelines: