Skip to content

FIX: assert the sparse-Q equality check in test_ddp.py - #908

Open
Sanjays2402 wants to merge 2 commits into
QuantEcon:mainfrom
Sanjays2402:fix/ddp-sparse-q-missing-assert
Open

FIX: assert the sparse-Q equality check in test_ddp.py#908
Sanjays2402 wants to merge 2 commits into
QuantEcon:mainfrom
Sanjays2402:fix/ddp-sparse-q-missing-assert

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #874

The sparse-Q comparison in test_ddp_to_sa_and_to_product computed a boolean and threw it away, so that check has never verified anything — a wrong converted Q would pass silently. This adds the missing assert (and fixes the adjacent allcose typo).

The assertion passes against the current conversion code; substituting a deliberately wrong Q into the same expression raises AssertionError, confirming it now guards. pytest quantecon/markov/tests/test_ddp.py is green (20 passed).

The sparse transition-array comparison in test_ddp_to_sa_and_to_product
computed a boolean and discarded it, so the check never verified anything:
a wrong converted Q would not fail the test. Add the missing assert (and
fix the adjacent 'allcose' typo). The assertion passes on the current
conversion code; feeding it a deliberately wrong Q raises AssertionError.

Closes QuantEcon#874
Comment thread quantecon/markov/tests/test_ddp.py Outdated
@Sanjays2402

Copy link
Copy Markdown
Author

Applied, thanks - switched to assert_(...) so it matches the rest of the file (the assert_ usages at lines 62-63, 95-96, 348-350, 366-367). assert_ is already imported from numpy.testing, so no import change was needed. pytest quantecon/markov/tests/test_ddp.py passes: 20 passed.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 90.546% (-3.2%) from 93.725% — Sanjays2402:fix/ddp-sparse-q-missing-assert into QuantEcon:main

@oyamad oyamad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FIX: Sparse-Q equality check in test_ddp.py is missing its assert

3 participants