Add new test comparing hard-coded and implied rates #1164
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
@martinholmer raised issue #1159 about misleading documentation of the two Growth class parameters. @codykallen agreed (and #1160 fixed the documentation in
growth.json
), but asked why the values in theGrowth.REAL_GDP_GROWTH_RATES
list did not correspond to any recent CBO assumptions. @andersonfrailey said those list values were derived from a formula. When @martinholmer pointed out that the formula was not correct, @andersonfrailey agreed and said thattest_default_rates_and_those_implied_by_blowup_factors
should be reviewed because the formula was from there. And finally, @martinholmer found in the git history that the initial version of this test was introduced in #424, and that the values in the list (which was then calledGrowth.REAL_GDP_GROWTH
) were specified to pass the test.Pull Request
This pull request creates a new test that compares three sets of hard-coded rates with the rates implied by the information in the
StageIfactors.csv
file, the contents of which are read and transformed by theRecords._read_blowup
method called by the Records class constructor. Using the information in theBF
DataFrame to construct the implied rates is complicated, so the new test derives the implied rates in two different ways: (1) starting with theBF
information in the Records object (as done in the old test), and (2) starting from the information in theStageIfactors.csv
file. Deriving the implied rates in two ways and testing that the two sets of implied rates are exactly the same, raises confidence in the logic used to compute the implied rates.This new test shows that the implied rates in the old test were not exactly correct for two of the three hard-coded rates. The implied price inflation rates in the new and old test were exactly the same and also exactly the same as those hard-coded in
Policy.__pirates
. But there are differences between the new and old implied rates for the hard-codedGrowth.REAL_GDP_GROWTH_RATES
and for the hard-codedPolicy.__wgrates
.This pull request does nothing but introduce the new test, but the final comparison between the hard-coded and implied rates has been commented out in two of the three cases. Uncommenting these final comparisons will require changing the hard-coded rates and dropping the old test. If no concerns with this pull request arise, those changes will be accomplished in subsequent pull requests: one that changes the hard-coded
Growth.REAL_GDP_GROWTH_RATES
and another that changes the hard-codedPolicy.__wgrates
.It would be prudent to review this pull request quickly so that we can proceed to the subsequent pull requests. Unless somebody can point out a problem with the new test (and there may be one or more problems because the logic of deriving the implied rates is fairly complicated), I plan to merge it into the master branch on Tuesday, January 31st.