PolicyEngine / policyengine-us

The PolicyEngine US Python package contains a rules engine of the US tax-benefit system, and microdata generation for microsimulation analysis.
https://policyengine.org/us
GNU Affero General Public License v3.0
100 stars 174 forks source link

Why is the recently added DC reform always being created? #3097

Closed martinholmer closed 11 months ago

martinholmer commented 11 months ago

@nikhilwoodruff recently added a dc_tax_threshold_joint_ratio_reform class to PE-US. This reform seems to be created every time a Microsimulation object is used to calculate any state income tax liability. This does not seem correct. Can this be fixed?

Here is an example of what I'm seeing:

PASSED IA/j21 test
PASSED IA/i21 test
PASSED IA/h21 test
PASSED IA/k21 test
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
PASSED IL/w21 test
PASSED IL/x21 test
PASSED IL/v21 test
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
creating dc_tax_threshold_joint_ratio_reform <class 'policyengine_us.reforms.dc_tax_threshold_joint_ratio.create_dc_tax_threshold_joint_ratio_reform.<locals>.reform'>
PASSED IN/x21 test
PASSED IN/v21 test
PASSED IN/w21 test
nikhilwoodruff commented 11 months ago

Thanks for raising Martin- taking a look now

nikhilwoodruff commented 11 months ago

OK- just to confirm that this bug does not affect simulation results: it actually just prints that it's constructed the DC reform should this be needed to be applied. I'll remove this unnecessary print statement set in #3096.

martinholmer commented 11 months ago

@nikhilwoodruff said:

OK- just to confirm that this bug does not affect simulation results: it actually just prints that it's constructed the DC reform should this be needed to be applied.

OK, but why is this construction always being done even when the model is going to do nothing with DC income taxes (much less a structural reform of DC income taxes)? Seems like a very inefficient way of doing things.

nikhilwoodruff commented 11 months ago

It's an issue with PolicyEngine Core that's fixable. Essentially, it's because we have some YAML tests that require reforms to be applied to test that reformed policy logic functions as expected, and PolicyEngine Core lets you do this by specifying a path to a Python module that has the reform defined and instantiated. The Python module we currently point to is also imported by the central tax benefit system so the instantiation happens every time. We could fix this by either:

martinholmer commented 11 months ago

@nikhilwoodruff, Thanks for the informative explanation in issue #3097.

It does seem like some kind of efficiency enhancement to the Core would be highly desirable. This will be especially important as we finish all the state income taxes and the number of structural reforms of different state income taxes starts to grow.