Skip to content

[lqcontrol] style guide review and updated maths#613

Open
matheusvillasb wants to merge 7 commits into
mainfrom
edit-lqcontrol
Open

[lqcontrol] style guide review and updated maths#613
matheusvillasb wants to merge 7 commits into
mainfrom
edit-lqcontrol

Conversation

@matheusvillasb

Copy link
Copy Markdown
Contributor

-Proofread, checked all links and references
-Updated maths to style guide
-Added a few comments to solve questions I had while reading the document

Comment thread lectures/lqcontrol.md Outdated
@mmcky

mmcky commented Sep 12, 2025

Copy link
Copy Markdown
Contributor

@matheusvillasb -- @HumphreyYang has answers to the questions you asked here #609 (comment) from #609.

@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-613--sunny-cactus-210e3e.netlify.app (53352eb)

📚 Changed Lecture Pages: lqcontrol

@mmcky mmcky removed the in-work label Sep 17, 2025
@matheusvillasb matheusvillasb marked this pull request as ready for review September 17, 2025 00:23
Comment thread lectures/lqcontrol.md Outdated
@mmcky

mmcky commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

@matheusvillasb the syntax around links wasn't formatted correctly. Checkout https://manual.quantecon.org/styleguide/doclinks.html and a99a241

@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-613--sunny-cactus-210e3e.netlify.app (a99a241)

📚 Changed Lecture Pages: lqcontrol

@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-613--sunny-cactus-210e3e.netlify.app (cf57cac)

📚 Changed Lecture Pages: lqcontrol

@mmcky

mmcky commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

@matheusvillasb this lecture has quite a few style guide violations such as

Do you mind doing another pass? Note: there have been edits to this branch so you should git pull origin edit-lqcontrol before making any additional changes.

@HumphreyYang I see there is a model in this lecture. Do you think we should use NamedTuple (in similar style to the JAX conversions)?

@HumphreyYang

HumphreyYang commented Sep 17, 2025

Copy link
Copy Markdown
Member

@HumphreyYang I see there is a model in this lecture. Do you think we should use NamedTuple (in similar style to the JAX conversions)?

Many thanks @matheusvillasb and @mmcky,

Yes, I think it would be better by putting these argument into a class LQParams(NamedTuple) instead of globals:

r = 0.05
β = 1 / (1 + r)
T = 45
c_bar = 2
σ = 0.25
μ = 1
q = 1e6

@matheusvillasb

Copy link
Copy Markdown
Contributor Author

@matheusvillasb this lecture has quite a few style guide violations such as

Do you mind doing another pass? Note: there have been edits to this branch so you should git pull origin edit-lqcontrol before making any additional changes.

@HumphreyYang I see there is a model in this lecture. Do you think we should use NamedTuple (in similar style to the JAX conversions)?

@mmcky I'll do that! Thank you for picking these issues.

@matheusvillasb

matheusvillasb commented Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

@mmcky

Do you mind doing another pass? Note: there have been edits to this branch so you should git pull origin edit-lqcontrol before making any additional changes.

So just to be sure, I need to do this while on my branch, right? Not on main.

Also, I tried to create a new branch to edit the lecture that follows this one. When I type git status, this is what I get:

PS C:\Users\mathe\QuantEcon_Lectures_Improvement\lecture-python.myst> git status
On branch edit-lqcontrol
Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: lectures/lagrangian_lqdp.md

no changes added to commit (use "git add" and/or "git commit -a")

Is this correct? The impression I have is that the edits in lagrangian_lqdp.md were made in the branch for the lqcontrol lecture instead of the branch I created for lagrangian_lqdp.

@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-613--sunny-cactus-210e3e.netlify.app (63f62a2)

📚 Changed Lecture Pages: lqcontrol

@jstac

jstac commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Status note for a future session — from a maintainer investigation on 2026-07-08 into why open-PR previews 404. Context only, not instructions.

Netlify preview: https://pr-613--sunny-cactus-210e3e.netlify.app/ currently returns 404.

Why previews are down (repo-wide findings)

1. This branch is stale — 178 commits behind main. A preview build compiles the whole site from this branch. This branch's lectures/house_auction.md still has unpinned !pip install prettytable, which now breaks on a wcwidth incompatibility. main fixed this on 2026-06-28 by pinning prettytable<3.18 (#939). This alone fails any rebuild of this branch until it's updated to main.

2. The arviz failure was a red herring — do NOT pin arviz or rewrite plotting. A 2026-07-07 rebuild also failed in ar1_bayes/ar1_turningpts with an arviz_plots figsize ValueError. That was a transient bug in an intermediate arviz-plots 1.x release, already fixed in arviz 1.2.0. Verified locally on a clean latest-stack venv: the real az.plot_trace(trace) cell (pymc + numpyro InferenceData) runs green. The lectures use only 1.x-compatible arviz APIs (plot_trace, summary, from_numpyro, compare).

Recommended first step for this PR

Update this branch to main (merge or rebase — pulls in #939 plus ~178 other commits), then let CI rebuild. On today's latest libraries the site builds clean, so the preview should return. house_auction is the known blocker; updating also picks up other since-merged fixes — rebuild and address any remaining per-lecture failures. Verify with:

curl -sI https://pr-613--sunny-cactus-210e3e.netlify.app/lqcontrol.html

This PR touches: lqcontrol.md. Last CI build: success@2025-09-29. Branch: 178 commits behind main as of 2026-07-08.

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.

4 participants