BUG: Fix DiscreteDP input validation and iteration edge cases

[oyamad]

Closes #856.

This PR fixes several input-validation and edge-case bugs in DiscreteDP (quantecon/markov/ddp.py), found while reviewing the module against its QuantEcon.jl counterpart.

1. Duplicate state-action pairs silently corrupted the model

In the state-action pair formulation with unsorted indices, duplicate (s, a) pairs were not detected: coo_matrix(...).tocsr() sums duplicate entries, and since the entries here are position pointers into R and Q, the constructor either raised an uninformative IndexError or, when the summed pointer happened to land in bounds, silently built a corrupted instance (len(R) < num_sa_pairs, rewards and transition probabilities reassigned across pairs, stale s_indices) that then solved without complaint. A ValueError('duplicate state-action pair found') is now raised, consistent with the behavior of QuantEcon.jl. Example that previously constructed (and solved) silently:

s_indices = [1, 1, 0, 0]  # (1, 0) duplicated
a_indices = [0, 0, 0, 1]
R = [1., 2., 3., 4.]
Q = [(0.5, 0.5), (1., 0.), (0.25, 0.75), (0.6, 0.4)]
ddp = DiscreteDP(R, Q, 0.95, s_indices, a_indices)  # no error; wrong model

2. Trailing states with no action caused out-of-bounds reads

With sorted input indices, _generate_a_indptr walked s_indices past the end whenever the last state(s) had no action available — an unchecked out-of-bounds read under Numba's nopython mode. With unsorted input indices, the coo_matrix shape was inferred from the largest indices present, so a_indptr came out shorter than num_states+1, with the same consequence downstream. The kernel now bounds its walk and the COO shape is passed explicitly, so the intended informative ValueError ("for every state at least one action must be available") is raised in both cases. The two checks in _check_action_feasibility are also reordered (action availability first), since for a state with no action s_wise_max leaves the corresponding entry of R_max uninitialized.

3. operator_iteration returned a tuple when max_iter <= 0, and max_iter=0 crashed policy_iteration

operator_iteration returned (v, 0) instead of the documented num_iter on its early-return path. The four solution methods now raise ValueError for max_iter < 1 (previously policy_iteration died with NameError, and value_iteration stored the tuple in res.num_iter).

4. Cosmetics

beta is now validated at the top of __init__ (previously only after the index-sorting work); typo fixes in docstrings ("optinal", "represetned", "correspnoding", "feasbile") and in a test name.

Tests are added for each fix; the full quantecon/markov test suite passes (90 passed).

🤖 Generated with Claude Code (Claude Fable 5)

[coveralls]

coverage: 93.607% (+0.05%) from 93.561% — oyamad:ddp-input-validation into QuantEcon:main

[Copilot]

Pull request overview

This PR hardens DiscreteDP’s SA-pair constructor and iteration routines by adding missing input validation and fixing edge cases that could previously lead to silent model corruption or unsafe out-of-bounds access under Numba. It also aligns behavior more closely with the QuantEcon.jl implementation and adds targeted regression tests.

Changes:

• Prevent silent corruption by detecting duplicate (state, action) pairs during SA-pair index sorting and raising a clear ValueError.
• Fix trailing “no available actions” cases by bounding _generate_a_indptr’s index walk and by supplying an explicit COO shape so a_indptr is always length num_states + 1.
• Make iteration semantics consistent by returning an integer iteration count from operator_iteration for max_iter <= 0, and raising ValueError in all solve methods when max_iter < 1 (plus doc/test typo fixes).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
quantecon/markov/ddp.py	Adds early beta validation, detects duplicate SA pairs on CSR conversion, reorders feasibility checks to avoid uninitialized reads, fixes operator_iteration return type, and validates max_iter in solvers.
quantecon/markov/utilities.py	Bounds _generate_a_indptr’s index walk to handle trailing states with no SA pairs safely.
quantecon/markov/tests/test_ddp.py	Adds regression tests for trailing no-action states, duplicate SA pairs, and nonpositive max_iter, plus fixes a test name typo.

[mmcky]

Thanks so much for this, @oyamad! 🙏 Really nice to see these DiscreteDP edge cases rounded up and fixed in one go — every claim was easy to check. Approving.

All four fixes target genuine bugs I reproduced on main, the fixes are correct, and each new test fails on base / passes here (full quantecon/markov suite: 90 passed).

Verified independently:

• Duplicate (s,a) — nnz < num_sa_pairs is sound and complete: no false positives (tocsr() sums duplicates but keeps explicit zeros, so the arange-pointer 0 is not dropped), and duplicates always route to the unsorted branch because _has_sorted_sa_indices rejects equal adjacent pairs — so the sorted branch legitimately needs no check of its own. 👍
• Trailing no-action state — the unbounded _generate_a_indptr walk was genuine numba UB (nopython disables bounds-checking; on base I reproduced it yielding a corrupt non-monotonic a_indptr). The bound plus the explicit coo_matrix shape fix both the sorted and unsorted paths, and _generate_a_indptr was the only kernel in utilities.py with that unbounded pattern.
• operator_iteration / max_iter — return type now matches the num_iter : scalar(int) docstring; the four guards are correctly placed after the None-default; neither internal caller relied on the old tuple.
• beta move — behavior-neutral; nothing between the old and new position reads beta.

Nice touch confirming parity with QuantEcon.jl — the Julia constructor raises ArgumentError("Duplicate s-a pair found") on the same input, so the behaviors line up.

A few optional, non-blocking thoughts — purely take-it-or-leave-it:

• beta == 1 now emits its warning even when construction later fails structurally, since the warn moved above the structural checks. Harmless.
• The trailing-state test's sorted sub-case relies on numba UB failing on base (a bit non-deterministic); pytest.raises(ValueError, match='at least one action') would make it a firmer regression sentinel. A sparse-Q trailing case and a max_iter=-1 value would round out coverage nicely.
• Might be worth a changelog line for the operator_iteration return-type change and the new ValueErrors, since both are user-visible.

I also filed #856 for a related, out-of-scope gap I noticed while digging: the sorted branch still silently accepts out-of-range state indices, an asymmetry this PR's shape fix happens to surface (not a regression from your change). No action needed here — just opened it for discussion.

Thanks again 🎉

[mmcky]

thanks @oyamad I am happy for you to merge this any time you're ready.