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

Fix Sabelhaus-Song variances #980

Merged
merged 4 commits into from
Mar 3, 2021
Merged

Conversation

Mv77
Copy link
Contributor

@Mv77 Mv77 commented Mar 2, 2021

I made a grave error with the calibration tools. Sabelhaus & Song's model returns income shock variances, while HARK expects standard deviations. I did not transform the variances and thus, at the moment, shock variances are being interpreted as standard deviations. This can cause income uncertainty to be understated by a factor of 10.

This PR fixes the error. My sincerest apologies for this bug and its timing.

  • Tests for new functionality/models or Tests to reproduce the bug-fix in code.
  • Updated documentation of features that add new functionality.
  • Update CHANGELOG.md with major/minor changes.

@codecov-io
Copy link

Codecov Report

Merging #980 (05e6446) into master (799943e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #980   +/-   ##
=======================================
  Coverage   71.85%   71.85%           
=======================================
  Files          62       62           
  Lines        9205     9205           
=======================================
  Hits         6614     6614           
  Misses       2591     2591           
Impacted Files Coverage Δ
HARK/Calibration/Income/tests/test_IncomeTools.py 100.00% <ø> (ø)
HARK/Calibration/Income/IncomeTools.py 92.79% <100.00%> (ø)
...nsumptionSaving/tests/test_IndShockConsumerType.py 100.00% <100.00%> (ø)
...ptionSaving/tests/test_IndShockConsumerTypeFast.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 799943e...05e6446. Read the comment docs.

@llorracc
Copy link
Collaborator

llorracc commented Mar 2, 2021

Oops! Thanks for catching it -- just in time, we haven't released 0.10.9 yet, but are hoping to do so tonight or tomorrow.

PS. Maybe you don't need to update the changelog because you had updated it already with your earlier PR? If that's right, just tick off the "changelog" tick box, and I'll merge.

@Mv77
Copy link
Contributor Author

Mv77 commented Mar 2, 2021

That's great to hear. Yes, the tools were already in the changelog. I have simply added a link to this PR. It can be merged after tests complete (although they should pass).

Thank you and sorry again!

@Mv77
Copy link
Contributor Author

Mv77 commented Mar 3, 2021

All tests passed, this can now be safely merged!

@llorracc llorracc merged commit d99a98a into econ-ark:master Mar 3, 2021
@Mv77 Mv77 deleted the Calibration/SabSong_fix branch March 9, 2021 03:10
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