While reviewing lectures/inventory_q.md as part of mirroring it into QuantEcon/lecture-dp, Copilot flagged two issues in the same code block (create_sdd_inventory_model). Both look real and worth a small follow-up here.
Issue 1 — `D_MAX` upper-bound wording
def create_sdd_inventory_model(
K: int = 20, # Max inventory
D_MAX: int = 21, # Demand upper bound for summation
...
):
...
d_values = np.arange(D_MAX) # 0, 1, ..., D_MAX - 1
The comment calls `D_MAX` the demand upper bound, but `np.arange(D_MAX)` makes it an exclusive upper bound — the largest demand actually included is `D_MAX - 1` (= 20 by default).
Either tweak the wording (e.g. `# Demand upper bound (exclusive) for summation` or `# Demand support size`), or use `np.arange(D_MAX + 1)` if the inclusive reading was intended.
Issue 2 — truncated geometric does not sum to 1
def demand_pdf(p, d):
return (1 - p)**d * p
d_values = np.arange(D_MAX)
ϕ_values = demand_pdf(p, d_values) # ϕ_0, ϕ_1, ...
`(1-p)^d * p` is the geometric PMF on `d = 0, 1, 2, ...` and so `ϕ_values` over `d = 0..D_MAX-1` does not sum to 1 — the tail mass `(1-p)^{D_MAX}` is dropped. With defaults (`p = 0.7`, `D_MAX = 21`) the missing mass is `0.3^{21} ≈ 10^{-11}` — completely negligible numerically — but for smaller `p` it can become non-trivial, which would bias the Bellman expectation and the greedy policy.
One simple fix is to fold the tail into the last grid point:
ϕ_values = demand_pdf(p, d_values)
ϕ_values[-1] = 1.0 - ϕ_values[:-1].sum()
or otherwise renormalize / document the truncation explicitly.
Happy to send a small PR if useful. Not blocking anything downstream; `lecture-dp` is mirroring the file as-is so this stays a single-source fix.
🤖 Found via Copilot review on QuantEcon/lecture-dp#19
While reviewing
lectures/inventory_q.mdas part of mirroring it intoQuantEcon/lecture-dp, Copilot flagged two issues in the same code block (create_sdd_inventory_model). Both look real and worth a small follow-up here.Issue 1 — `D_MAX` upper-bound wording
The comment calls `D_MAX` the demand upper bound, but `np.arange(D_MAX)` makes it an exclusive upper bound — the largest demand actually included is `D_MAX - 1` (= 20 by default).
Either tweak the wording (e.g. `# Demand upper bound (exclusive) for summation` or `# Demand support size`), or use `np.arange(D_MAX + 1)` if the inclusive reading was intended.
Issue 2 — truncated geometric does not sum to 1
`(1-p)^d * p` is the geometric PMF on `d = 0, 1, 2, ...` and so `ϕ_values` over `d = 0..D_MAX-1` does not sum to 1 — the tail mass `(1-p)^{D_MAX}` is dropped. With defaults (`p = 0.7`, `D_MAX = 21`) the missing mass is `0.3^{21} ≈ 10^{-11}` — completely negligible numerically — but for smaller `p` it can become non-trivial, which would bias the Bellman expectation and the greedy policy.
One simple fix is to fold the tail into the last grid point:
or otherwise renormalize / document the truncation explicitly.
Happy to send a small PR if useful. Not blocking anything downstream; `lecture-dp` is mirroring the file as-is so this stays a single-source fix.
🤖 Found via Copilot review on QuantEcon/lecture-dp#19