QuantEcon / QuantEcon/QuantEcon.py

TST: test_left_eigen_vec's multi-distribution branch is never executed

Open
#961 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue low-priority tests
Dominant language
Python
Stars
2.4k
Forks
2.3k
Avg merge
3d 3h
Merged PRs (30d)
3

Description

Problem

Test_markovchain_stationary_distributions_KMRMarkovMatrix2.test_left_eigen_vec branches on self.n_stat_dists, but the class's only fixture is the KMR sequential-move matrix with N = 27. That chain is irreducible, so it has exactly one stationary distribution: self.stationary is always (1, 28) and n_stat_dists is always 1. The else branch at quantecon/markov/tests/test_core.py:230-233 therefore never executes.

The same applies to setup_method. stat_shape always has length 2, so the if len(stat_shape) == 1 arm at line 201 is dead too, and n_stat_dists is only ever assigned from stat_shape[0].

Nothing is broken here — the assertion that does run is the correct one for an irreducible chain. The gap is that the reducible, multiple-stationary-distribution case the else branch was written for is not covered anywhere in this class, so it is untested code masquerading as a test.

Noticed while reviewing #798, which rewrites both np.dot calls in this method to @. That PR is a faithful mechanical rewrite and the dead branch predates it, so this is a separate pre-existing gap rather than anything introduced there.

Verification

Against e7b86e5:

Fact Value
P.shape (28, 28)
mc.is_irreducible True
stationary_distributions.shape (1, 28)
len(stat_shape) 2 — so the setup_method if arm at line 201 is never taken
n_stat_dists 1 — so the test_left_eigen_vec else arm at lines 230-233 is never taken
Suggested fix

Add a reducible fixture so the branch actually runs. A four-state chain with two absorbing states yields n_stat_dists == 2:

P = np.array([[1.0, 0.0, 0.0, 0.0],
              [0.0, 1.0, 0.0, 0.0],
              [0.3, 0.3, 0.4, 0.0],
              [0.0, 0.2, 0.0, 0.8]])

Both stationary rows satisfy v @ P == v, so the existing assertion passes unchanged once the branch is reachable — the branch is correct, it is simply never entered.

The alternative, if the multi-distribution case is judged to be covered well enough by test_markovchain_pmatrices, is to delete the else branch and the dead setup_method arm instead. Either resolution is fine; leaving an unreachable assertion in place is the outcome worth avoiding.

Acceptance criteria
  • The else branch of test_left_eigen_vec is either exercised by a reducible fixture or removed
  • The dead if len(stat_shape) == 1 arm in setup_method is resolved the same way
  • Full suite still passes

Noticed during review of #798 (AI-assisted; claims verified against e7b86e5 on 2026-09-10).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in quantecon/markov/tests/test_core.py, especially setup_method and Test_markovchain_stationary_distributions_KMRMarkovMatrix2.test_left_eigen_vec. Check how the existing KMR fixture determines stat_shape and n_stat_dists, then use the proposed reducible fixture or remove the unreachable branches. Run the full test suite and confirm both acceptance criteria are resolved.

Written by the indexing model from the issue text.

Assessment

Tech stack
numpy, python
Domain
testing-qa
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.