Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flatten default initializer dictionaries #1425

Merged
merged 20 commits into from
May 21, 2024
Merged

Conversation

mnwhite
Copy link
Contributor

@mnwhite mnwhite commented May 15, 2024

This PR has (or should have) no functional changes. All it does is reorganize the default initializer dictionaries for our AgentTypes so that all parameters for that class are present and explained in one place. All parameter values remain the same, and the dictionaries themselves should be exactly identical as before.

This branch builds from #1410 under the assumption that it will be merged in relatively soon.

  • Update CHANGELOG.md with major/minor changes.

mnwhite added 3 commits May 15, 2024 16:05
PerfForesight and IndShock dictionaries now stand on their own. KinkedR is still defined from IndShock because it's in the *same file* and is *barely different*. Also changed the spacing / line breaks to better organize the parameters visually.
The KinkedR dictionary had an extraneous CubicBool=True in it, for no reason at all. Removing that changed the target test values in the third or fourth digit, so I've adjusted them here.
@mnwhite
Copy link
Contributor Author

mnwhite commented May 15, 2024

So I said it wouldn't change any dictionaries, but then I found an extraneous CubicBool=True and removed it. Test values changed by 0.0001 to 0.001.

mnwhite added 14 commits May 15, 2024 17:02
Of course the one thing I rename for consistency has its own test.
Also partway through ConsBequestModel, will finish on next commit.
Also fixes an import error in RiskyContribModel.
Still need to do the other dictionaries for AggShockModel, but made the main one flat.
And turns out there was basically nothing else to do in AggShockModel.
Import same dictionary from different file.
Because pLvlPctiles is now constructed rather than passed raw, the grid changed *very slightly*, so test values changed *very slightly*.
Tiny grid change --> tiny result change.
TBS didn't require any meaningful work, yay.
Had to rearrange how the tests are run. This commit will surely break a bunch of examples.
Adjusted base dictionary for one test, changed parameter formatting for the other. In the example notebook, custom built MrkvArray has to be passed to the AgentType instance *after* initialization, because MrkvArray now has a default constructor. This behavior can be improved on later once there's just one call to construct() (and improved dictionaries).
@mnwhite mnwhite changed the title WIP: Flatten default initializer dictionaries Flatten default initializer dictionaries May 18, 2024
@mnwhite
Copy link
Contributor Author

mnwhite commented May 18, 2024

This branch is ready for review and merge. It extends #1410 , so that should be merged first.

There are very few functional changes, mostly just "flattening" of dictionaries so that each AgentType subclass is better documented in the file in which it appears. A few tests and examples had to have their formatting changed slightly to account for dictionary differences. Only one test had a value change, and that was due to removing a (likely mistaken) CubicBool=True in an example dictionary.

The tests that changed will be easier to see when #1410 is merged into master.

@mnwhite mnwhite requested review from alanlujan91 and sbenthall May 18, 2024 17:29
@sbenthall
Copy link
Contributor

I'll review this once #1410 is in. As is, it's hard to isolate the changes specific to this PR

@sbenthall sbenthall merged commit 3eb9a09 into master May 21, 2024
18 checks 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.

2 participants