PolicyEngine / policyengine-core

Core microsimulation engine for PolicyEngine models. Forked from OpenFisca-Core.
https://policyengine.github.io/policyengine-core
GNU Affero General Public License v3.0
14 stars 20 forks source link

fix: Shallow copy group_entities and person_entity when cloning TBS #294

Closed anth-volk closed 2 weeks ago

anth-volk commented 2 weeks ago

Fixes #293.

Previous fixes to -core ensured that when a TaxBenefitSystem object was cloned via its internal .clone() method, entities themselves were shallow copied to prevent stale variable versions from being transferred between systems. However, this fix assumed that GroupEntity and PopulationEntity objects were included within the TaxBenefitySystem's entities key, when they are not.

This led structural reform runs to crash, as the GroupEntity and PopulationEntity would possess stale variable versions from the API's first instantiation of the default tax-benefit system, attempt to calculate using this, then crash when they couldn't find a variable that had been newly created as part of a structural reform.

This PR ensures that GroupEntity and PopulationEntity objects are also shallow-copied to prevent this behavior. This PR does not add tests at the moment, but I have opened an issue in the -api package laying out a comprehensive series of tests around structural reforms at https://github.com/PolicyEngine/policyengine-api/issues/1885. I have also opened a draft PR in the -us package to ensure these changes cause no test failures there (https://github.com/PolicyEngine/policyengine-us/pull/5240).

anth-volk commented 2 weeks ago

Temporarily moving to draft; I think we can actually re-enable the two tests removed previously, but want to confirm