ENH: Add informative ValueError for LQMarkov DARE non-convergence when beta=1

[HG-Cheng]

References

Addresses #508

Description

This PR improves the handling and diagnostics of LQMarkov problems around beta = 1.

Changes

• Preserve valid beta = 1 use cases in the plain LQ class.
• Reject beta > 1 when constructing LQMarkov.
• Allow LQMarkov(beta=1) to reach the Riccati solver.
• When the Riccati system does not converge within max_iter and beta == 1, raise a more informative ValueError explaining that strict contraction is not guaranteed and that convergence may be very slow or fail to reach the requested tolerance.
• Add regression tests covering:
  • valid plain LQ construction with the default beta=1;
  • LQMarkov(beta=1) construction;
  • rejection of LQMarkov(beta>1);
  • the informative beta=1 solver failure message.

No changes are made to the Riccati iteration algorithm itself.

Validation

The changes were tested after merging the latest upstream main:

• test_lqcontrol.py: 12 passed
• test_matrix_eqn.py: 4 passed
• test_lqnash.py: 2 passed
• Full test suite: 603 passed

[HG-Cheng]

Hello maintainers,

I noticed the CI pipeline failed on the macos-latest runner.

Looking closely at the logs, the failure is entirely isolated to quantecon/util/tests/test_timing.py (ACTUAL: 0.203895 vs DESIRED: 0.05). It appears to be a transient flaky test caused by CI runner CPU load fluctuation, as this PR only modifies a string message in _matrix_eqn.py and does not touch any timing utility logic.

Just leaving a note here for visibility. Looking forward to your review on the core changes!

[oyamad]

@HG-Cheng Thank you for the contribution!

Do you know what is known to happen when beta > 1? (The current code does not prohibit beta > 1.)
I think it is better to either change if beta == 1.0 to if beta >= 1.0 or reject beta > 1 upon construction.

[HG-Cheng]

@oyamad "Thanks for the review!
You are right, $\beta > 1$ doesn't make economic sense in this context. I agree that rejecting $\beta > 1$ upon construction is the cleaner approach. I will update the PR to include this validation and push the changes shortly."

[oyamad]

@HG-Cheng Next question is: what is known to happen when beta = 1?

For the instance in #508 (comment), it does not converge even with max_iter=1_000_000:

ValueError: Convergence failed after 1000000 iterations.

[HG-Cheng]

Hi @oyamad,Thanks for running that test! That actually makes perfect sense mathematically.If $\beta = 1$, there is absolutely no discounting. This means future costs don't decay over time. If we sum them up over an infinite horizon, the total simply diverges to infinity.Because the mathematical result is infinite, the underlying Riccati solver is essentially trying to compute a finite limit that doesn't exist. That's exactly why it spins endlessly and fails even after 1,000,000 iterations—it's looking for a convergent solution where there isn't one.Given this, I have updated the validation from if beta > 1.0: to if beta >= 1.0: in the latest commit. This way, we can "fail-fast" and reject $\beta = 1$ upon construction as well, preventing the solver from wasting compute time on an endless loop.Let me know if you agree and if we are good to merge!

[mmcky]

Hi @HG-Cheng — thanks so much for digging into this, and for the great back-and-forth with @oyamad in the thread! 🙌 The improved, context-aware message in solve_discrete_riccati_system is genuinely useful and is exactly the kind of guidance #508 was asking for.

There's one thing I think we should sort out before this is ready, though: the new construction-time guard is a bit too broad and would break valid uses of the plain LQ class.

The change adds this to both LQ.__init__ and LQMarkov.__init__:

if beta >= 1.0:
    raise ValueError("Discount factor beta cannot be greater than 1.")

For LQ, beta=1 is actually the documented default, and it's perfectly valid in two common cases:

• Finite-horizon problems (when T is set) — undiscounted LQ is standard and shows up all over the QuantEcon lectures.
• Deterministic infinite-horizon problems (C=0).

LQ already rejects beta >= 1 in the one case where it's genuinely invalid — stochastic infinite-horizon — over in _lqcontrol.py (lines 147–149):

if (self.C != 0).any() and beta >= 1:
    raise ValueError('beta must be strictly smaller than 1 if ' +
        'T = None and C != 0.')

So the new guard would make even LQ(Q, R, A, B) with the default beta=1 raise on construction, which we'd want to avoid. (The existing test_lqcontrol.py cases all use beta=0.95, so the suite won't flag it, but a lot of lecture code relies on the beta=1 default.)

The "β=1 diverges" reasoning holds specifically for the LQMarkov DARE system — which is what #508 is about — but not for the plain LQ finite-horizon path, which solves by backward induction and handles beta=1 explicitly.

A possible path forward:

1. Drop the beta >= 1.0 guard from LQ.__init__ — the existing check already covers the invalid case there.
2. Keep the improved fail_msg in solve_discrete_riccati_system — that's the real win here. 👍
3. For LQMarkov, if we want a construction-time guard, it might be cleaner to limit it to beta > 1 (which is economically nonsensical) and still let beta = 1 reach the solver so the new informative message can guide the user. Worth a quick confirmation with @oyamad on whether to fail fast at beta = 1 or lean on the improved message.

Two tiny nits in _matrix_eqn.py while you're in there: a new blank line picked up some trailing whitespace, and one comment got de-indented from 4 to 3 spaces — easy tidy-ups.

Thanks again for the contribution — this is close, and the diagnostic message is a real improvement! Let me know if anything's unclear. 😊

[HG-Cheng]

Thanks for the detailed feedback, @mmcky!

I've updated the PR accordingly:

• removed the broad beta >= 1 guard from plain LQ, preserving its valid beta=1 use cases;
• limited the LQMarkov construction-time check to beta > 1;
• kept beta=1 on the solver path and retained the improved diagnostic message, with more cautious wording;
• fixed the whitespace/indentation nits;
• added regression tests for the relevant LQ, LQMarkov, and solver behavior.

I also merged the latest upstream main and reran the full test suite: 603 tests passed.

Thanks again!