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

Update OG-USA environment #322

Merged
merged 3 commits into from
Oct 27, 2017
Merged

Update OG-USA environment #322

merged 3 commits into from
Oct 27, 2017

Conversation

jdebacker
Copy link
Member

@jdebacker jdebacker commented Oct 27, 2017

This PR updates the OG-USA environment to be consistent with the environment used for other PolicyBrain tools.

A few adjustments were made to the demographics.py script to work with the updated version of NumPy.

The SS works fine. I am still running the full tax function estimation and time path equilibrium. Hold off on merging until those are reviewed.

@hdoupe
Copy link
Contributor

hdoupe commented Oct 27, 2017

@jdebacker This looks good. Thanks for the quick response on this issue.

@jdebacker
Copy link
Member Author

Update: The time path solution completed and results look very similar to what we had before (differences from prior runs appear small enough that I think they are due to updating from Tax-Calc 0.8.3 to 0.12.0).

If this satisfied the environment for PolicyBrain, I will merge. @hdoupe let me know.

@hdoupe
Copy link
Contributor

hdoupe commented Oct 27, 2017

This is great. Thanks again for the quick response. This should be fine. I looked through the changes you made and they appear pretty minor. Which reform did you use to run the model?

I added a quick test to compare the macro output data in PR #316 . If you used a reform that was used in the regression tests, we could compare the results and be a little more certain that nothing broke in the update.

@jdebacker
Copy link
Member Author

@hdoupe I just ran the baseline. I did get the exact same results using old tax functions, so I think we are ok. In my run over night, the tax functions would have changed (slightly) due to changes in tax-calculator.

I'll go ahead and merge.

@jdebacker jdebacker merged commit 4f823b8 into PSLmodels:master Oct 27, 2017
@jdebacker jdebacker deleted the pins branch October 27, 2017 15:20
@hdoupe
Copy link
Contributor

hdoupe commented Oct 27, 2017

That makes sense. Thanks

@rickecon
Copy link
Member

I see @talumbau adding and removing "in progress" labels to these issues. Much appreciated. I have to believe that TJ has set up a GitHub bot on his account to do this. Otherwise, TJ is a very vigilant issue label maintainer.

@andersonfrailey
Copy link
Contributor

I see @talumbau adding and removing "in progress" labels to these issues. Much appreciated. I have to believe that TJ has set up a GitHub bot on his account to do this. Otherwise, TJ is a very vigilant issue label maintainer.

Personally, I like the believe it's the latter.

keiirizawa pushed a commit to keiirizawa/OG-USA that referenced this pull request Mar 20, 2019
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.

None yet

5 participants