Skip to content

ENH: Add state_values to DiscreteDP#832

Open
HG-Cheng wants to merge 5 commits into
QuantEcon:mainfrom
HG-Cheng:feature-ddp-values
Open

ENH: Add state_values to DiscreteDP#832
HG-Cheng wants to merge 5 commits into
QuantEcon:mainfrom
HG-Cheng:feature-ddp-values

Conversation

@HG-Cheng

@HG-Cheng HG-Cheng commented May 29, 2026

Copy link
Copy Markdown

This PR partially addresses #248 by adding state_values support to
DiscreteDP.

Changes:

  • Add an optional state_values argument to DiscreteDP.
  • Validate state_values consistently with MarkovChain.
  • Preserve state_values when converting between product and
    state-action-pair formulations.
  • Pass state_values to the MarkovChain returned by controlled_mc.
  • Add tests for storage, validation, propagation, and representation
    conversion.

action_values and num_actions are intentionally left out of this PR,
following the review discussion.

@coveralls

coveralls commented May 30, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 92.722% (+0.02%) from 92.706% — HG-Cheng:feature-ddp-values into QuantEcon:main

@oyamad

oyamad commented May 30, 2026

Copy link
Copy Markdown
Member

@HG-Cheng Thank you for the contribution!

Main comments

  1. I propose that we only add state_values and remove action_values (and num_actions) in this PR.

    Please see Adding State-Action Pair Support QuantEcon.jl#94 (comment):
    We are implicit which of the two approaches in that comment we are following (although I believe approach 1 is the legitimate approach). We need a discussion to decide on which approach to pick (or even whether to pick one explicitly). I propose not to discuss it here, focusing only on adding state_values in this PR.

  2. Please pass state_values=state_values to MarkovChain in controlled_mc: https://github.com/HG-Cheng/QuantEcon.py/blob/fff5405ae7d4f66df60bb2ae39a943504be19c59/quantecon/markov/ddp.py#L1004
    This is where adding state_values is most useful.

Other comments

Please do not include changes in the files .vscode/settings.json and _matrix_eqn.py in this PR.

@HG-Cheng

Copy link
Copy Markdown
Author

"Hi @oyamad, thanks for the detailed review and guidance!

I completely understand. I will remove action_values and num_actions to keep this PR focused on state_values only, and we can leave the state-action pair discussion for the future.

I will pass state_values=state_values to MarkovChain in controlled_mc as suggested.

Apologies for the accidental inclusion of .vscode/settings.json and _matrix_eqn.py! They leaked in from my local workspace and another task. I will remove them from this branch right away.

I'll push the updated commits shortly!"

@HG-Cheng HG-Cheng changed the title ENH: Add state_values and action_values to DiscreteDP (Fixes #248) ENH: Add state_values to DiscreteDP Jul 14, 2026
@HG-Cheng

Copy link
Copy Markdown
Author

Thanks again for the review. I've updated the PR to keep it focused on
state_values only, removed the unrelated changes, and propagated
state_values to the controlled MarkovChain.

I also synced the branch with the latest main. The full local test suite passes locally (605 passed).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants