search

PSLmodels / Business-Taxation

USA Corporate and Pass-Through Business Tax Model
11 stars 10 forks source link

Fix bug in reform1 specification #75

Closed martinholmer closed 5 years ago

martinholmer commented 5 years ago

This pull request fixes an incorrect depreciation method specified in test reform1. This change was indicated by @codykallen in this comment.

@codykallen, is it your expectation that changing this causes no difference in the test results? Why would GDS and Economic depreciation methods produce exactly the same results?

codykallen commented 5 years ago

@martinholmer asked:

Why would GDS and Economic depreciation methods produce exactly the same results?

The type was 'EconomicDS' for some reason. If it said 'Economic', it would have used economic depreciation. Because the typo version was not one of 'ADS', 'Economic' or 'None', the default system of GDS was used. Replacing 'EconomicDS' with GDS will not change the results, but replacing 'EconomicDS' with 'Economic' should change the results.

martinholmer commented 5 years ago

@codykallen said in PR #75:

but replacing 'EconomicDS' with 'Economic' should change the results

But the test results didn't change. What's going on?

codykallen commented 5 years ago

@martinholmer, I think it would be best to just change the reform dictionary to use 'depr_25yr_method': 'GDS'. We can open a separate issue to figure out what's going wrong in the Asset class.

martinholmer commented 5 years ago

@codykallen said in PR #75:

I think it would be best to just change the reform1 dictionary to use 'depr_25yr_method': 'GDS'. We can open a separate issue to figure out what's going wrong in the Asset class.

OK, can you open that new issue referring to the unexpected no-change results in this closed pull request?

I'll make the change you suggest in another pull request and in the other pending pull requests.

martinholmer commented 5 years ago

PR #75 has been closed in favor of PR #78.