Skip to content

Commit

Permalink
random function in formulas.py only applies randomness to some da…
Browse files Browse the repository at this point in the history
…ta, creating visual bug within `policyengine-app`

Fixes #134
  • Loading branch information
nikhilwoodruff committed Dec 14, 2023
1 parent d79b051 commit f4d7aba
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
4 changes: 4 additions & 0 deletions changelog_entry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- bump: patch
changes:
fixed:
- Bug causing random behaviour differences between situations with and without axes.
9 changes: 6 additions & 3 deletions policyengine_core/commons/formulas.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from policyengine_core.types import ArrayLike, ArrayType
from policyengine_core.variables.variable import Variable

import json

T = TypeVar("T")


Expand Down Expand Up @@ -298,10 +300,11 @@ def amount_between(
return clip(amount, threshold_1, threshold_2) - threshold_1


def random(entity, reset=True):
def random(entity, reset=False):
if entity.simulation.has_axes:
# Don't simulate randomness in simulations with axes.
return 0
# Generate the same random number for each entity.
random_number = np.random.rand(1)[0]
return np.array([random_number] * entity.count)

Check warning on line 307 in policyengine_core/commons/formulas.py

View check run for this annotation

Codecov / codecov/patch

policyengine_core/commons/formulas.py#L306-L307

Added lines #L306 - L307 were not covered by tests
if reset:
np.random.seed(0)
x = np.random.rand(entity.count)
Expand Down
12 changes: 12 additions & 0 deletions policyengine_core/simulations/simulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
TracingParameterNodeAtInstant,
)

import json

if TYPE_CHECKING:
from policyengine_core.taxbenefitsystems import TaxBenefitSystem

Expand Down Expand Up @@ -154,6 +156,16 @@ def __init__(
if len(self.get_holder(variable.name).get_known_periods()) > 0
]

self.situation_input = situation
if self.situation_input is not None:
original_input = json.loads(json.dumps(self.situation_input))

Check warning on line 161 in policyengine_core/simulations/simulation.py

View check run for this annotation

Codecov / codecov/patch

policyengine_core/simulations/simulation.py#L161

Added line #L161 was not covered by tests
if original_input.get("axes") is not None:
original_input["axes"] = {}

Check warning on line 163 in policyengine_core/simulations/simulation.py

View check run for this annotation

Codecov / codecov/patch

policyengine_core/simulations/simulation.py#L163

Added line #L163 was not covered by tests
# Hash the situation input to a random number, so situations with axes behave the
# same ways as the same situations without axes.
hashed_input = hash(json.dumps(original_input)) % 1000000
np.random.seed(hashed_input)

Check warning on line 167 in policyengine_core/simulations/simulation.py

View check run for this annotation

Codecov / codecov/patch

policyengine_core/simulations/simulation.py#L166-L167

Added lines #L166 - L167 were not covered by tests

def apply_reform(self, reform: Union[tuple, Reform]):
if isinstance(reform, tuple):
for subreform in reform:
Expand Down

0 comments on commit f4d7aba

Please sign in to comment.