Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

rs_inventory_q / inventory_q: misleading 'Converged' print + snapshot off-by-one in Q-learning loop

未关闭
#882 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
3/5
预计耗时
1-2 天
新手友好度
68/100
Issue 类型
缺陷
描述清晰度
基本清楚
活跃度
冷清
技术栈
python
领域
documentation

调研方向

先查看 lectures/rs_inventory_q.md 中的 solve_rs_inventory_model 和 q_learning_rs_kernel,然后比较 lectures/inventory_q.md 中对应的 solve 函数以及“Running Q-learning”部分。检查循环终止消息、快照时机,以及 n 与 n+1 的调用语义。完成的标准是:两个讲义文件都准确报告未收敛,并且文字、快照索引和已执行的步数一致。

由索引模型根据 Issue 内容生成。

描述

While reviewing lectures/rs_inventory_q.md as part of mirroring it into QuantEcon/lecture-dp, Copilot flagged three things — all real, none blocking, but worth a small follow-up here.

Issue 1 — `solve_rs_inventory_model` prints "Converged" unconditionally

`rs_inventory_q.md` (and the same pattern in `inventory_q.md` line 331):

def solve_rs_inventory_model(v_init, model, max_iter=10_000, tol=1e-6):
    v = v_init.copy()
    i, error = 0, tol + 1
    while i < max_iter and error > tol:
        new_v = T_rs(v, model)
        error = np.max(np.abs(new_v - v))
        i += 1
        v = new_v
    print(f"Converged in {i} iterations with error {error:.2e}")
    ...

If the loop exits because `i == max_iter` (before `error <= tol`), the print still claims convergence. With the default 10k cap that won't bite at the configured parameters, but it's misleading for anyone who tweaks settings.

Suggestion: only print "Converged" when `error <= tol`, otherwise warn (or raise). Same fix applies in `inventory_q.md`.

Issue 2 — snapshot recorded at the start of the loop iteration

`rs_inventory_q.md` (`q_learning_rs_kernel`):

for t in range(n_steps):
    # Record policy snapshot if needed
    if snap_idx < n_snaps and t == snapshot_steps[snap_idx]:
        snapshots[snap_idx] = greedy_policy_from_q_rs(q, K)
        snap_idx += 1
    ...
    q[x, a] = (1 - α) * q[x, a] + α * target
    ...

The snapshot at index `t` captures `q` before the update for step `t`. So a "snapshot at step n" actually reflects `q` after `n` completed updates only if `t = n` is visited after the n-th update — which requires running `n + 1` iterations.

That's exactly what the caller does, but it's load-bearing for the narrative…

Issue 3 — narrative vs. code mismatch in "Running Q-learning"

`rs_inventory_q.md` line 658:

We run $n$ = 5 million steps and take policy snapshots at steps 10,000, 1,000,000, and $n$.

n = 5_000_000
snap_steps = np.array([10_000, 1_000_000, n], dtype=np.int64)
q_table, snapshots = q_learning_rs(model, n_steps=n+1, snapshot_steps=snap_steps)

`n_steps=n+1` is the workaround for Issue 2 — without it, the snapshot at step `n` would not capture the final update. Two ways to make the narrative consistent:

  1. Move the snapshot recording to after the Q update (or after the loop), then call with `n_steps=n`.
  2. Or keep the snapshot at the start of the iteration but adjust indices so the caller passes `n_steps=n` and `snapshot_steps=[..., n-1]` etc.

Option 1 is cleaner and would let the prose stay literal ("run `n` steps").


Happy to send a small PR if useful — none of these are blocking and `lecture-dp` is mirroring as-is so this stays a single-source fix.

🤖 Found via Copilot review on QuantEcon/lecture-dp#20

主要语言
TeX
星标
125
派生
57
平均合并
1 天 22 小时
30 天内合并 PR
18

环境准备

这个项目没有提供开发容器、Dockerfile 或贡献指南,环境需要你自己搭建:先看它的 README,通用步骤见我们的新手贡献指南。

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

QuantEcon/lecture-python.myst 的其他 Issue

查看 QuantEcon/lecture-python.myst 的全部 Issue

相似的 Issue

更多 Documentation Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。