From 6b28cdd11a87ae30ab637a88a53e5a70479b821d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 6 Jul 2026 14:41:28 -0400 Subject: [PATCH] Preserve bool dtype in SingleAmountTaxScale.calc() Boolean single_amount brackets (amount_unit: bool, amount: true/false) previously returned an int64 0/1 array from .calc() because the sentinel padding [0] + self.amounts + [0] coerced the whole array to int64. Applying ~ (logical NOT) to that int array performed a bitwise negation (~1 == -2, ~0 == -1, both truthy) instead of logical NOT, silently producing wrong results in consuming formulas. Build the amounts array from the bracket amounts first so numpy infers their natural dtype, then pad the out-of-range sentinel cells with a zero of that same dtype (False for bool, 0 for numeric). Numeric int/float brackets are unchanged; empty scales still return int64 [0]. Fixes PolicyEngine/policyengine-us#8780 Co-Authored-By: Claude Fable 5 --- changelog.d/bool-bracket-dtype.fixed.md | 1 + .../taxscales/single_amount_tax_scale.py | 23 +++- .../test_single_amount_tax_scale.py | 111 ++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 changelog.d/bool-bracket-dtype.fixed.md diff --git a/changelog.d/bool-bracket-dtype.fixed.md b/changelog.d/bool-bracket-dtype.fixed.md new file mode 100644 index 000000000..e572dfa92 --- /dev/null +++ b/changelog.d/bool-bracket-dtype.fixed.md @@ -0,0 +1 @@ +`SingleAmountTaxScale.calc()` now preserves the bracket amounts' dtype, so boolean `single_amount` brackets (`amount_unit: bool`) return a boolean array instead of an int64 `0`/`1` array; this restores logical NOT (`~`) on the result, which previously performed a bitwise negation and silently produced wrong values. diff --git a/policyengine_core/taxscales/single_amount_tax_scale.py b/policyengine_core/taxscales/single_amount_tax_scale.py index 52effc785..9c1eed67d 100644 --- a/policyengine_core/taxscales/single_amount_tax_scale.py +++ b/policyengine_core/taxscales/single_amount_tax_scale.py @@ -19,6 +19,15 @@ def calc( """ Matches the input amount to a set of brackets and returns the single cell value that fits within that bracket. + + The returned array preserves the dtype of the bracket amounts. In + particular, a scale whose amounts are booleans (``amount_unit: bool`` + with ``amount: true``/``false``) returns a boolean array rather than the + int64 ``0``/``1`` array that naive sentinel padding would produce. This + keeps logical NOT (``~``) working on the result: ``~`` on an int array + is a bitwise negation (``~1 == -2`` and ``~0 == -1``, both truthy), + whereas ``~`` on a bool array is the intended element-wise logical + negation. """ guarded_thresholds = numpy.array([-numpy.inf] + self.thresholds + [numpy.inf]) @@ -28,6 +37,18 @@ def calc( right=right, ) - guarded_amounts = numpy.array([0] + self.amounts + [0]) + # Build the amounts array from the bracket amounts first, so numpy + # infers their natural dtype (bool for boolean brackets, int/float + # otherwise), then pad the two out-of-range sentinel cells with a zero + # of that same dtype (``False`` for bool, ``0`` for numeric). Padding + # with a Python int ``0`` up front, as the previous implementation did, + # coerced the whole array to int64 and silently dropped a boolean dtype. + # With no amounts, fall back to the historical int64 zero array. + if len(self.amounts) == 0: + guarded_amounts = numpy.array([0, 0]) + else: + amounts = numpy.asarray(self.amounts) + sentinel = numpy.zeros(1, dtype=amounts.dtype) + guarded_amounts = numpy.concatenate([sentinel, amounts, sentinel]) return guarded_amounts[bracket_indices - 1] diff --git a/tests/core/tax_scales/test_single_amount_tax_scale.py b/tests/core/tax_scales/test_single_amount_tax_scale.py index 907bbe1f1..0c5db35ce 100644 --- a/tests/core/tax_scales/test_single_amount_tax_scale.py +++ b/tests/core/tax_scales/test_single_amount_tax_scale.py @@ -45,6 +45,117 @@ def test_to_dict(): assert result == {"6": 0.23, "9": 0.29} +def test_calc_preserves_boolean_dtype(): + # A single_amount scale whose amounts are booleans (amount_unit: bool) + # must return a boolean array, not an int64 0/1 array. Otherwise logical + # NOT (~) silently becomes a bitwise negation on the result. + tax_scale = taxscales.SingleAmountTaxScale() + tax_scale.add_bracket(0, True) + tax_scale.add_bracket(16, False) + + result = tax_scale.calc(numpy.array([10.0, 20.0])) + + assert result.dtype == numpy.bool_ + assert result.tolist() == [True, False] + + +def test_calc_boolean_result_supports_logical_not(): + # The regression this guards: ~ on the int64 output was a bitwise negation + # (~1 == -2, ~0 == -1, both truthy), so negating a bool bracket produced + # wrong results. On a real bool array, ~ is the intended logical NOT. + tax_scale = taxscales.SingleAmountTaxScale() + tax_scale.add_bracket(0, True) + tax_scale.add_bracket(16, False) + + result = tax_scale.calc(numpy.array([10.0, 20.0])) + negated = ~result + + assert negated.dtype == numpy.bool_ + assert negated.tolist() == [False, True] + + +def test_calc_preserves_integer_dtype(): + # Numeric int amounts must be unaffected by the boolean-dtype fix: the + # result stays int64 with identical values. + tax_scale = taxscales.SingleAmountTaxScale() + tax_scale.add_bracket(0, 100) + tax_scale.add_bracket(16, 200) + + result = tax_scale.calc(numpy.array([10.0, 20.0])) + + assert result.dtype == numpy.int64 + assert result.tolist() == [100, 200] + + +def test_calc_preserves_float_dtype(): + # Numeric float amounts must be unaffected: the result stays float64. + tax_scale = taxscales.SingleAmountTaxScale() + tax_scale.add_bracket(6, 0.23) + tax_scale.add_bracket(9, 0.29) + + result = tax_scale.calc(numpy.array([1, 8, 10])) + + assert result.dtype == numpy.float64 + tools.assert_near(result, [0, 0.23, 0.29]) + + +def test_calc_out_of_range_bool_returns_false_sentinel(): + # Inputs that fall outside every bracket must get the zero-valued sentinel + # in the amounts' own dtype: False for a boolean scale, not int 0. + tax_scale = taxscales.SingleAmountTaxScale() + # digitize guards with [-inf, threshold..., inf]; the -inf..first-threshold + # cell is a real bracket, so use a scale where only interior cells map to a + # bracket and confirm the sentinel dtype anyway via a below-all base. + tax_scale.add_bracket(0, True) + + result = tax_scale.calc(numpy.array([-5.0])) + + assert result.dtype == numpy.bool_ + + +def test_calc_empty_scale_stays_int(): + # An empty scale has no amounts to infer a dtype from; preserve the + # historical behaviour of returning an int64 zero array. + tax_scale = taxscales.SingleAmountTaxScale() + + result = tax_scale.calc(numpy.array([10.0])) + + assert result.dtype == numpy.int64 + assert result.tolist() == [0] + + +def test_calc_boolean_dtype_end_to_end_via_parameter_scale(): + # Mirror how policyengine-us declares boolean single_amount brackets + # (amount_unit: bool, amount: true/false) and confirm the dtype survives + # the full ParameterScale -> get_at_instant -> SingleAmountTaxScale path. + data = { + "description": "Boolean age-exemption single_amount scale", + "metadata": { + "type": "single_amount", + "threshold_unit": "year", + "amount_unit": "bool", + }, + "brackets": [ + { + "threshold": {"2022-01-01": {"value": 0}}, + "amount": {"2022-01-01": {"value": True}}, + }, + { + "threshold": {"2022-01-01": {"value": 16}}, + "amount": {"2022-01-01": {"value": False}}, + }, + ], + } + scale = parameters.ParameterScale("bool_scale", data, "") + scale_at_instant = scale.get_at_instant(periods.Instant((2022, 6, 1))) + + result = scale_at_instant.calc(numpy.array([10.0, 20.0])) + + assert result.dtype == numpy.bool_ + assert result.tolist() == [True, False] + assert (~result).tolist() == [False, True] + + # TODO: move, as we're testing Scale, not SingleAmountTaxScale def test_assign_thresholds_on_creation(data): scale = parameters.ParameterScale("amount_scale", data, "")