-
-
Notifications
You must be signed in to change notification settings - Fork 53
[ar1_bayes] Removed PyMC dependency and style-sheet compliance #569
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @HumphreyYang -- just one minor comment re: citation. Should I organise @thomassargent30 to review?
There was a problem hiding this 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 removes PyMC as a dependency from the AR(1) Bayesian analysis lecture, simplifying the codebase to use only NumPyro for Bayesian inference. The changes modernize variable naming conventions and clean up the implementation.
Key changes:
- Removed all PyMC imports, installations, and model implementations
- Updated variable naming to use Greek letters (ρ, σ) instead of Latin equivalents
- Fixed minor typos and improved code consistency
Co-authored-by: Copilot <[email protected]> Co-authored-by: Matt McKay <[email protected]>
Thanks Matt, I think it's fine. This is a minor change : ) |
thanks @HumphreyYang are you happy for me to merge this? |
@mmcky Where do I find the latest deployment? I can see above "🚀 Deployed on https://68bb28185ee35015bb2cce79--nostalgic-wright-5fa355.netlify.app/" I'm guessing this is out of date because it's not compliant with the syle manual (e.g., sentences running together instead of being followed by line breaks). I find this PR a bit hard to jump into quickly because I'm unsure of the deployment and the title of the lecture is missing. |
Many thanks @jstac, please see the latest deployment of the lecture Posterior Distributions for AR(1) Parameters: We will include the lecture title and the latest deployment of the lecture next time we ping you for review. Many thanks in advance! (I pushed some more updates since I found some more paragraphs with multiple sentences) |
@HumphreyYang doing some testing of the |
@copilot I have attached our style-guide quantecon_review_instructions.md Can you please review this lecture Are you able to do a full review of this lecture and not just a review of the changes. |
@HumphreyYang this sort of triggering doesn't work. I think it will only work when a PR is authored initially by |
Many thanks @mmcky, this PR should be compliant since I have run the same instruction on my end, but please let me know if your run spots anything new! |
thanks @HumphreyYang -- you're right. I'll open an issue to figure out how we can improve the |
Many thanks @mmcky! It's interesting that it posts deployment this time : ) |
@HumphreyYang it looks like |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
📖 Netlify Preview Ready! Preview URL: https://pr-569--sunny-cactus-210e3e.netlify.app (7e829d7) 📚 Changed Lecture Pages: ar1_bayes |
Key changes:
rho, sigma
in NumPyro code