Skip to content

Convert one-bracket rate scales without UnboundLocalError - #588

Open
MaxGhenis wants to merge 5 commits into
masterfrom
fix-singleton-tax-scale-conversion
Open

MaxGhenis wants to merge 5 commits into
masterfrom
fix-singleton-tax-scale-conversion

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

MarginalRateTaxScale.to_average() and LinearAverageRateTaxScale.to_marginal() read an unbound loop variable on singleton inputs, raising UnboundLocalError. The terminal rate now comes from the first bracket or last processed pair. See master's marginal conversion and average conversion. No policy source applies to this mathematical engine repair.

This round fixes singleton zero-tax calculation with constant rates at the origin and infinity, guarded by len(thresholds) == len(rates) == 1 and thresholds[0] == 0. It restores master's surplus-rate compatibility and restricts the no-exceptions invariant. The witness (-100,0.1),(0,0.2) still reproduces ZeroDivisionError; saved manual evidence establishes that it lies outside the supported guarantee.

Caller behavior

  • On master, marginal bracket (0, 0.37) raises on conversion. It now returns average thresholds [0, inf], rates [0.37, 0.37], and tax [0, 0.37, 0.555, 370] at bases [0, 1, 1.5, 1000]. Arithmetic: 1000 × 0.37 = 370; 1.5 × 0.37 = 0.555. The previous PR head returned [0, inf], [0, 0.37], whose infinite-width slope was zero and therefore levied zero tax.
  • Average bracket (0, 0.37) previously raised; it now converts to marginal thresholds [0], rates [0.37], with the same tax amounts at those nonnegative bases.
  • Empty average conversion previously raised; it now returns an empty marginal scale. Empty marginal conversion remains a zero-rate average scale with thresholds [0], rates [0].
  • Previously successful multibracket conversions retain master's results, including public-field assignments thresholds=[0,100], rates=[0.1,0.2,0.8]: both conversions retain terminal rate 0.2, rather than the previous PR head's 0.8. Metadata and input scales remain unchanged. Neither calculation method is modified.
  • Outside the guarantee, nonzero-origin marginal singletons retain the reviewed head's [0, inf], [0, r], calculating zero at finite bases. Average-singleton conversion still starts at 0 regardless of the stored threshold. These cases gain no broader calculation guarantee.

Methodology

  • Retain master's knot-preserving contract: average-to-marginal preserves threshold tax and the terminal whole-base rate. Preserving continuously interpolated average tax at every interior base is an alternative and generally needs varying marginal rates. This PR retains master's multibracket results: average knots (0,0),(1,0.1),(2,0.2) produce marginal rates [0.1,0.3,0.2], since (0.4−0.1)/(2−1)=0.3; converted tax at 1.5 is 0.1 + 0.5×0.3 = 0.25, versus interpolated average tax 1.5×0.15 = 0.225.
  • For surplus rates, preserve master's last paired rate. Taking the last stored rate or rejecting mismatches are alternatives; this round restores master compatibility.
  • A zero-origin marginal singleton has constant average rate for nonnegative bases. Retain [0, inf] with both rates r; a one-knot average representation would produce negative tax at negative bases. This supported conversion changes from master's exception to a functioning result; negative marginal bases still yield zero.

Invariants and validation

Hypothesis covers equal-length scales with 1–6 brackets, rates in [0,1], first threshold 0, and strictly increasing positive integer upper thresholds bounded by 1,000,000: conversions do not raise; marginal round-trips preserve thresholds/rates and the infinity terminal knot; average-to-marginal preserves threshold tax and terminal whole-base tax. Additional properties check singleton calculation at finite bases within ±1,000,000 and multibracket conversion compatibility when surplus rates are appended.

Before fixing the review findings, the new singleton calculation regression failed (1 failed, 19 passed) and its property failed (1 failed, 2 passed). The surplus-rate regressions also failed before repair: 2 marginal tests, 4 average tests, and both generated conversion cases. Expectations include hand arithmetic in the regression comments.

Final files run individually: test_marginal_rate_tax_scale.py: 22 passed; test_linear_average_rate_tax_scale.py: 14 passed; test_rate_scale_conversion_property.py: 5 passed, each with 300-example settings. 41 tests passed, none skipped. Ruff format check: 300 files already formatted; Ruff lint and git diff --check: passed. Full suites and microsimulations were excluded by the assignment. Fetched gh/master at 78a6c503 has no overlapping tax-scale changes, so no master merge was made. Core contains no production conversion calls; the independent review reported no known country callers. Documentation review: impact low, confidence high within the stated domain; autogenerated API references need no change. Unsupported domains remain excluded. The changelog is updated.

MarginalRateTaxScale.to_average and LinearAverageRateTaxScale.to_marginal
took the top rate from a loop variable that only the second and later
brackets set, so a one-bracket scale (for example threshold 0, rate 0)
raised UnboundLocalError in both directions. Both now use the scale's own
last rate, which is the value that loop variable held for every scale with
two or more brackets, so those conversions are unchanged. An empty
linear-average scale converts to an empty marginal scale.

Adds single-bracket and empty regressions, plus Hypothesis properties:
marginal -> average -> marginal returns the original brackets (one to six
brackets from threshold 0), and to_marginal keeps the tax an average scale
levies at its thresholds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis and others added 4 commits October 6, 2026 16:59
From the independent review of ad577d5 (approved, three nits):

- `if not self.rates` raised for a NumPy array of rates (truth value of
  an array is ambiguous), which master accepted. Use len() instead.
- The to_marginal property compared with LinearAverageRateTaxScale.calc
  below the last threshold only, which left two-bracket scales checking
  nothing but tax 0 at 0 (a mutant setting the first marginal rate to 999
  survived). It now checks the definition directly: the marginal scale
  levies threshold x rate at every threshold, and the top rate on the
  whole base above the last one. That mutant is now killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The independent review of 99dbcd1 found the new property fails every
time under Hypothesis's CI profile (derandomized, so --reruns cannot
recover it) and about 2% of random runs: brackets
([0, 1, 999517, 999518], [0, 0, 0.005, 0]) were off by 1.2e-6 against
atol=1e-6. MarginalRateTaxScale.calc scales thresholds by 1 + eps, which
moves the tax by about t x eps x the bracket's rate, and close thresholds
near a million make that rate large. The tolerance now adds
8 x eps x (sum of rate x threshold + the base above the last threshold).

Under CI=true both properties pass; 0 of 40 random seeds fail; the
review's failing cases pass; the first-rate-999 mutant is still killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant