a not-too-general value backwards induction algorithm built on DBlocks#1438
a not-too-general value backwards induction algorithm built on DBlocks#1438sbenthall wants to merge 21 commits intoecon-ark:mainfrom
Conversation
|
Clarifying that by "general", I mean "not-too-general". A "not-too-general" solver, by my definition, is a solver that:
This is in contrast with a "model specific solver", which is what most HARK models currently have; these solvers contain specific model data. |
| "b": lambda k, R, PermGroFac: k * R / PermGroFac, | ||
| "m": lambda b, theta: b + theta, | ||
| "c": Control(["m"]), | ||
| "c": Control(["m"], upper_bound=lambda m: m), |
There was a problem hiding this comment.
@mnwhite @alanlujan91 I wonder what you think about this way of introducing upper/lower bound information on control variables.
There was a problem hiding this comment.
I think this looks good.
If we wanted to differentiate between a fixed (real number) upper bound, and a functional upper bound, we could use the term upper_envelope for a function/lambda.
…ng test on normalized consumption block
|
This PR now includes:
This PR is now ready for review. Requesting review from @alanlujan91 or @mnwhite As I mentioned in the original post for this PR, my next step, building on this PR, is a demonstration of a configuration that will use:
|
|
One thought on how to improve this PR:
|
|
Recent changes add handling and tests for cases where the stage has zero control variables. |
|
This is now ready for review again. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a general value backwards induction (VBI) algorithm that operates on DBlock objects, enabling solution of dynamic programming problems through grid-based optimization. The implementation serves as a foundation for API design of solution algorithms and demonstrates integration with the HARK model framework.
Changes:
- Adds
HARK.algos.vbimodule with asolvefunction implementing value backwards induction on DBlocks - Extends the
Controlclass to support upper and lower bounds on control variables - Adds
get_controls()method and enhancestransition()method in DBlock with ascreenparameter - Updates model definitions to include upper bounds on control variables
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| HARK/algos/vbi.py | New module implementing value backwards induction algorithm with grid-based optimization |
| HARK/algos/tests/test_vbi.py | Test suite for VBI algorithm with multiple test cases |
| HARK/model.py | Extended Control class with bounds and enhanced DBlock transition method |
| HARK/models/consumer.py | Added upper bounds to control variables and switched to normalized blocks |
| HARK/models/perfect_foresight.py | Added upper bound to consumption control |
| HARK/models/perfect_foresight_normalized.py | Added upper bound to consumption control |
| HARK/models/test_models.py | Removed unused initial state "p" from test configurations |
| Documentation/CHANGELOG.md | Added changelog entry for VBI algorithm |
| Documentation/reference/index.rst | Added algos module to documentation index |
| Documentation/reference/tools/algos.rst | New documentation file for algos module |
| Documentation/reference/tools/algos/vbi.rst | New documentation file for vbi submodule |
| Documentation/reference/tools/index.rst.orig | Merge conflict file that should be removed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # if no controls, no optimization is necessary | ||
| pass |
There was a problem hiding this comment.
When there are no control variables (len(controls) == 0), the code does nothing and proceeds with uninitialized policy_data and value_data. This will likely cause issues when trying to use these variables later. Consider either handling this case properly or raising an informative error.
| # if no controls, no optimization is necessary | |
| pass | |
| # if no controls, no optimization is necessary; directly evaluate value | |
| dr_best = {} | |
| value = srv_function(pre_states, dr_best) | |
| value_data.sel(**state_vals).variable.data.put(0, value) |
| ) | ||
|
|
||
|
|
||
| class test_vbi(unittest.TestCase): |
There was a problem hiding this comment.
The class name test_vbi does not follow Python naming conventions for test classes, which should use PascalCase (e.g., TestVbi or TestVBI). This inconsistency may cause issues with some test discovery tools.
| class test_vbi(unittest.TestCase): | |
| class TestVbi(unittest.TestCase): |
| "k": Lognormal(-6, 0), | ||
| #'live' : 1, | ||
| "p": 1.0, | ||
| # "p": 1.0,- not needed, using normalized problem |
There was a problem hiding this comment.
Missing space after the comment marker. Should be "# "p": 1.0, - not needed" instead of "#"p": 1.0,- not needed" for consistency with Python style guidelines.
| # "p": 1.0,- not needed, using normalized problem | |
| # "p": 1.0, - not needed, using normalized problem |
| pre_states.update(state_vals) | ||
|
|
||
| # prepare function to optimize | ||
| def negated_value(a): # old! (should be negative) |
There was a problem hiding this comment.
The comment "old! (should be negative)" is unclear and potentially confusing. If this is outdated information, it should be removed. If it's meant to explain why the function is negated, consider clarifying the comment to say something like "negated for use with minimization optimizer".
|
|
||
| bounds = ((lower_bound, upper_bound),) | ||
|
|
||
| res = minimize( # choice of |
There was a problem hiding this comment.
The comment "# choice of" on line 141 is incomplete. It appears to be a placeholder or unfinished thought that should either be completed or removed.
| res = minimize( # choice of | |
| res = minimize( # optimize the (bounded) scalar control using scipy.optimize.minimize |
| dr_from_data = { | ||
| c: ar_from_data( | ||
| policy_data | ||
| ) # maybe needs to be more sensitive to the information set | ||
| for i, c in enumerate(controls) | ||
| } |
There was a problem hiding this comment.
All controls share the same policy_data array in the decision rule construction. For problems with multiple controls, each control should have its own policy array. This will cause issues when the code is extended to handle multiple controls (currently it raises an exception for len(controls) > 1).
| dr_from_data = { | |
| c: ar_from_data( | |
| policy_data | |
| ) # maybe needs to be more sensitive to the information set | |
| for i, c in enumerate(controls) | |
| } | |
| if len(controls) == 0: | |
| # no controls: empty decision rule | |
| dr_from_data = {} | |
| elif len(controls) == 1: | |
| # single control: construct rule from the corresponding policy data | |
| dr_from_data = {controls[0]: ar_from_data(policy_data)} | |
| else: | |
| # multiple controls are not yet supported in value backup iteration | |
| raise Exception( | |
| f"Value backup iteration is not yet implemented for stages with {len(controls)} > 1 control variables." | |
| ) |
| - Introduces `HARK.parser' module for parsing configuration files into models [#1427](https://github.com/econ-ark/HARK/pull/1427) | ||
| - Allows construction of shocks with arguments based on mathematical expressions [#1464](https://github.com/econ-ark/HARK/pull/1464) | ||
| - YAML configuration file for the normalized consumption and portolio choice [#1465](https://github.com/econ-ark/HARK/pull/1465) | ||
| - Introduces value backup algorithm on DBlocks [#1438]https://github.com/econ-ark/HARK/pull/1438) |
There was a problem hiding this comment.
Missing opening parenthesis in the markdown link. The link should be formatted as [#1438](https://github.com/econ-ark/HARK/pull/1438) but is currently missing the opening parenthesis before https.
| - Introduces value backup algorithm on DBlocks [#1438]https://github.com/econ-ark/HARK/pull/1438) | |
| - Introduces value backup algorithm on DBlocks [#1438](https://github.com/econ-ark/HARK/pull/1438) |
|
|
||
| dr_best = {c: get_action_rule(res.x[i]) for i, c in enumerate(controls)} | ||
|
|
||
| policy_data.sel(**state_vals).variable.data.put(0, res.x[0]) # ? |
There was a problem hiding this comment.
The question mark comment "# ?" on line 162 suggests uncertainty about this code. If there's a known issue or concern, it should be documented more clearly. If not, the comment should be removed.
| policy_data.sel(**state_vals).variable.data.put(0, res.x[0]) # ? | |
| policy_data.sel(**state_vals).variable.data.put(0, res.x[0]) |
| iset : list of str | ||
| The labels of the variables that are in the information set of this control. | ||
|
|
||
| upper_bound : function |
There was a problem hiding this comment.
The docstring is incomplete. It only documents iset and upper_bound parameters, but the __init__ method also accepts a lower_bound parameter that should be documented.
| upper_bound : function | |
| lower_bound : function, optional | |
| An 'equation function' which evaluates to the lower bound of the control variable. | |
| upper_bound : function, optional |
| # def setUp(self): | ||
| # pass | ||
|
|
||
| def test_solve_block_1(self): | ||
| state_grid = {"m": np.linspace(0, 2, 10)} | ||
|
|
||
| dr, dec_vf, arr_vf = vbi.solve(block_1, lambda a: a, state_grid) | ||
|
|
There was a problem hiding this comment.
This comment appears to contain commented-out code.
| # def setUp(self): | |
| # pass | |
| def test_solve_block_1(self): | |
| state_grid = {"m": np.linspace(0, 2, 10)} | |
| dr, dec_vf, arr_vf = vbi.solve(block_1, lambda a: a, state_grid) | |
| def test_solve_block_1(self): | |
| state_grid = {"m": np.linspace(0, 2, 10)} | |
| dr, dec_vf, arr_vf = vbi.solve(block_1, lambda a: a, state_grid) | |
| dr, dec_vf, arr_vf = vbi.solve(block_1, lambda a: a, state_grid) |
This PR aims to provide a general backwards induction algorithm that operates on a single DBlock.
It simply arranges the pieces that come with the DBlock (decision value function, arrival value function), in a sensible way, with an optimization over the action space to find the best action at each point in the state grid.
While the algorithm is not as good as FOC or EGM methods, I'm doing this for several reasons:
The first commit of this PR just sketches the algorithm out, but it isn't tested.
One question is how to design this so that it is more agnostic to different optimizers. I know @alanlujan91 is keen on estimagic.