Conversation
The `get_economy_data` method on various AgentType subclasses in ConsAggShockModel has been deprecated in favor of a single `AgentType.get_market_params` method. Parameters to be fetched should be named in the class attribute `market_params` on the `AgentType` subclass, and the `Market` instance should run its `give_agent_params` method. I might have that be automatically called as part of instantiation. I also cleaned up the default dictionaries a little bit on this commit, and added an extremely small feature to the `get_it_from` special constructor. The example notebook runs, but the test file has not been edited. Also need to revise the documentation to explain the new (easier) format.
Two tests fail with slight changes to solution results. Committing and will initiate a PR so I can look carefully at what's changed.
Didn't see before that this one was failing.
The result changes are because of random seed changes. Essentially, the agents' distributions are produced in a different sequence than before (because they don't do an initial build) so the seed that is set for each distribution is different.
|
The two changed test targets are because the seeds of distributions have changed. With the revisions here, the "order of operations" for building distributions has changed, so the agents' |
If an argument of `calc_dynamics` is not named in `track_vars`, then the code now looks for it as an attribute of the market. This improves compatibility with `calc_dynamics` being passed as a function but still using parameters from the market.
Notebooks now have correct descriptions of `give_agent_params()`.
Market parameter structureMarket parameter structure
|
This is ready for review, but the CHANGELOG-only PR should be merged into main first, then pulled into this branch (and updated). |
|
CHANGELOG has been updated and this is now properly ready for review. |
There was a problem hiding this comment.
Pull request overview
This PR addresses issues #766 and #989 by regularizing how AgentType subclasses obtain parameters from their associated Market instances. The changes replace custom get_economy_data() methods with a unified get_market_params() approach that uses a market_vars class attribute to specify which parameters should be fetched from the Market.
Changes:
- Introduced
AgentType.get_market_params()method andmarket_varsclass attribute to replace customget_economy_data()methods across agent types - Added
Market.give_agent_params()convenience method to distribute market-level parameters to all agents and run theirconstruct()methods - Extended the
get_it_fromconstructor to handle scalar values directly, not just objects and dictionaries - Modified
Market.update_dynamics()to allowcalc_dynamicsto accept additional arguments beyond those intrack_varsby looking for matchingMarketattributes - Refactored income shock distribution constructors into standalone functions in
IncomeProcesses.py - Updated test assertion values that changed slightly due to different ordering of distribution construction
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| HARK/core.py | Added get_market_params() method to AgentType and give_agent_params() method to Market; enhanced update_dynamics() to fetch non-tracked arguments from Market attributes |
| HARK/utilities.py | Extended get_it_from constructor to return scalar values directly when the parent is a primitive type |
| HARK/ConsumptionSaving/ConsAggShockModel.py | Major refactoring: removed get_economy_data() and add_AggShkDstn() methods; added market_vars attributes; moved income shock combination logic to standalone constructor functions; renamed MrkvNow_init to MrkvInit for consistency |
| HARK/ConsumptionSaving/ConsIndShockModel.py | Updated docstring parameter names from aNrmInitMean/Std to kLogInitMean/Std and pLvlInitMean/Std to pLogInitMean/Std |
| HARK/Calibration/Income/IncomeProcesses.py | Removed duplicate assertion; added combine_ind_and_agg_income_shocks() and combine_markov_ind_and_agg_income_shocks() utility functions |
| tests/ConsumptionSaving/test_ConsAggShockModel.py | Updated tests to use give_agent_params() instead of manual loops calling get_economy_data(); updated expected test values that changed slightly due to constructor ordering changes |
| tests/ConsumptionSaving/test_SmallOpenEconomy.py | Replaced manual parameter assignment and get_economy_data() loop with consolidated dictionary approach and give_agent_params() call |
| examples/Journeys/Journey-PhD.ipynb | Updated to use new give_agent_params() method; incidentally changed Python version to 3.12.9 |
| examples/Gentle-Intro/Market-Intro.ipynb | Added documentation for max_loops parameter; clarified that calc_dynamics can accept additional Market attribute arguments; enhanced explanation of intractability in Krusell & Smith model; incidentally changed Python version to 3.12.9 |
| examples/Gentle-Intro/Constructors-Intro.ipynb | Added new section explaining the special case for AgentType subclasses in Market contexts that don't run construct() at instantiation |
| docs/CHANGELOG.md | Documented breaking changes and new features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This cell isn't run by default so I missed it
This PR is meant to address issues #766 and #989 ; the main change is a regularization of
get_economy_data. Rather than eachAgentTypesubclass having their own method that says how to get information from the associatedMarketinstance, there is now a unifiedAgentType.get_market_paramsmethod.AgentTypesubclasses that (usually) use an associateMarketshould specify amarket_varsclass attribute naming the parameters that should be pulled down from theMarket. The user also doesn't need to write a trivial loop overAgentTypeinstances in theMarket; instead, they can just callMarket.give_agent_params(), which also runsagent.construct()by default because the agents usually need information from the economy to finish building themselves.The example notebook has been lightly edited to work with the changes, and most of the tests pass. However, two market-level solution results changed slightly. I am issuing the PR now so I can look carefully at what I changed and see if I made a mistake somewhere.
Incidentally, this PR also makes a tiny extension to the special
get_it_fromconstructor. If the thing that it is told toget_it_fromis just a single float, int, bool, complex, or string, then it is interpreted as literally just taking that value itself. In most cases,get_it_fromis used to "unpack" attributes from an object or entries from a dictionary, but now it also means "it's this number by a different name".Tomorrow morning I'll add some flexibility to allow "parameter-like" arguments to
calc_dynamics, which is in a similar vein to these changes.