SESIT: Industry carbon tax - #94
Conversation
|
Before I read in detail -- are you sure the negative OBPS case flows smoothly? That there are no leaks, etc? |
jose-moran
left a comment
jose-moran
left a comment
There was a problem hiding this comment.
minor comments for the moment, I'll check in more detail later
| """ | ||
| self.extra_marginal_taxes_firm = np.zeros(self.firms.n_industries) | ||
|
|
||
| if self.use_obps_reg and self.obps is not None: |
There was a problem hiding this comment.
should you not throw an error or a warning if this function is called but these things are not set?
There was a problem hiding this comment.
Added a logging.warning when use_obps_reg=True but self.obps is None, so misconfiguration is visible at runtime instead of silently doing nothing.
| emission_limit: np.ndarray | ||
| price: np.ndarray | ||
| current_t: int = 0 | ||
| current_year: int = 2014 |
There was a problem hiding this comment.
Renamed reference_emission_intensity → baseline_emission_intensity to make clear it's the fixed 2017–2019 historical baseline, not a current value. Also added initial_year as an explicit field so reset() and the price-loading loop no longer hardcode 2014.
| self.industries = industries | ||
|
|
||
| # Industries regulated under OBPS (federal schedule) | ||
| self.regulated_industries = [ |
There was a problem hiding this comment.
is there a check for the case where you are running this with other industries and you still are using this obps? An error should be thrown I think
There was a problem hiding this comment.
Added two warnings on init:
If some regulated industries are missing from the model's industry list → logs which ones were skipped
If none of the regulated industries match at all → logs that OBPS will have no effect
…tion - Warn when use_obps_reg=True but no OBPS object is set (country.py) - Floor extra_marginal_taxes_firm at -good_prices so a negative rebate cannot push the effective sector price below zero (country.py) - Rename reference_emission_intensity -> baseline_emission_intensity to make clear the value is fixed from the 2017-2019 reference period - Add initial_year field so reset() and price-loading no longer hardcode 2014; current_year remains the mutable simulation state - Warn on init when regulated industries are absent from the model's industry set, with details on which codes were skipped - Update tests to reflect the baseline_emission_intensity rename
Made this addition: |
jose-moran
left a comment
jose-moran
left a comment
There was a problem hiding this comment.
Review: Canada OBPS carbon tax
Reviewed for correctness, numerical safety, style, tests, and -- the main thing -- how cleanly the Canada specifics are compartmentalized vs. how easily the generic machinery generalizes.
Compartmentalization: good. All Canada specifics (the regulated industry codes, the 2017/2019/2022 phase boundaries, the OBPS formula, the jurisdiction price lookup) are sealed inside OutputBasedPriceSystemCAN + OBPSCANReader. The engine only ever sees a generic extra_marginal_taxes_firm array, and reusing the existing extra_taxes hook in the price-setters was the right instinct.
Generalizability: not quite yet. There is no policy-instrument interface, so the generic code still names the concrete Canada scheme in three shared files (use_obps_reg, country.update_extra_taxes(), simulation.iterate()). A thin PolicyInstrument base would make a second country a drop-in. Details inline on country_configuration.py.
Style: clean. ruff format --check and ruff check both pass, no in-function imports, Pydantic config correct, thorough docstrings. (Note: the project CLAUDE.md still says CI uses black+isort, but the repo runs ruff only -- worth fixing the doc.)
Blocker: the price-array build crashes on the real sparse-year carbon-price schedule (inline on output_based_price_system_can.py). The unit tests pass only because the two test layers use different df_rates formats and never connect.
Other majors: Country.reset() does not reset OBPS state (breaks calibration/SBI re-runs); 2019 is double-purposed as both a reference and a levied year; the integration path is untested.
Inline comments below with specifics and suggested fixes. Well-documented and well-tested-at-the-unit-level overall -- these are mostly about real-data robustness and the generalization interface.
| df_sub = self.df_rates[["Date", self.country_name]] | ||
| for t in range(len(self.df_rates)): | ||
| df_row = df_sub[df_sub["Date"] == t + self.initial_year] | ||
| self.price[t] = df_row[self.country_name].values[0] |
There was a problem hiding this comment.
Blocker -- crashes on the real (sparse-year) carbon price schedule. This loop assumes Date is contiguous from initial_year: for index t it looks up Date == t + 2014. But Canada's schedule is milestone-based and the reader's own fixture is sparse (test_obps_can_reader.py:5 uses 2014, 2019, 2030). With sparse years, t=1 looks for Date==2015, gets an empty frame, and .values[0] raises IndexError. The unit tests pass only because the policy test hand-builds a contiguous range(2014, 2052) frame and never feeds reader output through this constructor.
Suggest indexing the price series by year (set Date as index, reindex over range(initial_year, max_year+1) with ffill/interpolation) and looking up by year offset, not row count.
| self.reference_emission += input_em + capital_em | ||
| self.reference_production += production | ||
|
|
||
| if self.current_year == 2019: |
There was a problem hiding this comment.
Major -- 2019 is both a reference year and a levied year. At current_year == 2019 we accumulate into the reference (L168-170), recompute the baseline from the running sums here, then fall through (2019 < 2019 is False) and levy a real cost at L183-189 -- against a benchmark that includes this same period's emissions. The PR text says reference = 2017-2019 but limits apply "from 2019 onwards", so 2019 is double-purposed. Suggest finalizing the baseline at the 2019->2020 transition and levying from >= 2020, or documenting the intent explicitly.
|
|
||
| def get_price(self) -> float: | ||
| """Return the current period carbon price ($/tCO₂e).""" | ||
| return self.price[self.current_t] |
There was a problem hiding this comment.
Minor -- get_price() is unclamped (self.price[self.current_t]) while compute_obps clamps with min(current_t, len(price)-1) at L189. Past the end of the schedule this IndexErrors. Currently unused so low risk, but the two should agree -- clamp here too.
| use_obps_reg=self.use_obps_reg, | ||
| record_obps_reference=record_obps_reference, | ||
| production=self.firms.ts.current("production"), | ||
| input_em=self.firms.ts.current("inputs_emissions") + self.firms.ts.current("inputs_emissions_ch4"), |
There was a problem hiding this comment.
Worth checking -- CH4 fields may be None. This sums inputs_emissions + inputs_emissions_ch4 (and the capital equivalents). If the CH4 fields can be None when add_emissions is True (firms set them to None when CH4 isn't configured), then array + None raises. Guard with a zero fallback if that combination is reachable.
|
|
||
| for country in self.countries.values(): | ||
| if country.obps is not None: | ||
| while country.obps.current_year < self.timestep.year: |
There was a problem hiding this comment.
Generalization smell. This year-advance special-cases the Canada .obps attribute inside the generic iterate loop. With a policy-instrument interface (see the comment on country_configuration.py) this becomes for p in country.policy_instruments: p.advance_to(self.timestep.year) and the engine never names a country-specific scheme. Also note the while loop silently assumes annual steps -- worth an assert or comment.
| assume_zero_noise: bool = False | ||
| use_emission_multiplier: bool = False | ||
| CH4_production_emissions_only: bool = False | ||
| use_obps_reg: bool = False |
There was a problem hiding this comment.
Generalization -- the main thing to address before this is reusable. Canada specifics are nicely sealed in the _can class/reader (good), but the generic machinery still names the concrete scheme in three shared files: this use_obps_reg boolean, country.update_extra_taxes() (hardwired to self.obps), and simulation.iterate(). Adding a second country today means editing all three.
A thin PolicyInstrument base (marginal_tax_by_sector(), advance_to(year), reset()) plus a country.policy_instruments: list[...] would make a new scheme = one new class + reader + config entry, with zero shared-file edits. Given the PR is framed as an "Example Canada Implementation", this interface is what makes that framing true -- happy to take it as a fast-follow if you'd rather merge the Canada case first.
| ratio = self._normalised_price(industry_name, current_quarter=current_time) | ||
| for idx in self._indices_for(industry_name): | ||
| price[idx] = base_prices[idx] * ratio | ||
| price[idx] = base_prices[idx] * ratio + tax_by_firm[idx] |
There was a problem hiding this comment.
Two notes on how the tax enters price-setting:
- The additive application here (
base_prices*ratio + tax) is theSectorExogenousPriceSetter. InDefaultPriceSetterthe tax only shiftsaverage_price_by_firm, which feeds the cost-push ratio -- so for default sectors a positive carbon tax can actually nudge the price down via the cost-push channel, and the-good_pricesfloor is largely inert there. Worth confirming the intended sign of the price response for non-exogenous sectors. ExogenousPriceSetter.compute_pricedidn't get theextra_marginal_taxeskwarg that the siblings did; if OBPS is ever combined with that setter,firms.compute_priceforwarding the kwarg willTypeError. Add it (ignored) for signature consistency.
| reduction_factor: float = REDUCTION_FACTOR, | ||
| tightening_rate: float = TIGHTENING_RATE, | ||
| ) -> OBPSCANData: | ||
| df_rates = pd.DataFrame({"Date": list(range(2014, 2052)), "CAN": [carbon_price] * 38}) |
There was a problem hiding this comment.
Test gap. This fixture builds a fully contiguous range(2014, 2052) price frame, which is what hides the sparse-year IndexError in the constructor (see output_based_price_system_can.py:137). Two gaps to close: (1) a test that runs the reader's sparse-year output through OutputBasedPriceSystemCAN(...), and (2) an integration test through Country.update_extra_taxes() covering the per-unit divide, the -good_prices floor, and the two warning paths -- none of which are currently exercised.
|
|
||
| from macro_data.readers.policy_data.obps_can_reader import OBPSCANReader | ||
|
|
||
| RATES_CSV = "Date,CAN,CAN_BC\n2014,15.0,12.0\n2019,40.0,30.0\n2030,170.0,150.0\n" |
There was a problem hiding this comment.
This sparse-year fixture (2014, 2019, 2030) is realistic, but it is never fed into OutputBasedPriceSystemCAN, whose constructor assumes contiguous years and would IndexError on exactly this data. An end-to-end reader->policy test using this CSV would catch it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Module and compute_obps docstrings claimed max(0, emissions - limit) but the implementation computes the signed difference with no clamp — sectors below their benchmark receive a rebate (negative cost). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers OutputBasedPriceSystemCAN (positive cost, negative rebate, tightening rate, reference accumulation, disabled/pre-2019 guards) and OBPSCANReader (CSV loading, missing files, optional elec file). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion - Warn when use_obps_reg=True but no OBPS object is set (country.py) - Floor extra_marginal_taxes_firm at -good_prices so a negative rebate cannot push the effective sector price below zero (country.py) - Rename reference_emission_intensity -> baseline_emission_intensity to make clear the value is fixed from the 2017-2019 reference period - Add initial_year field so reset() and price-loading no longer hardcode 2014; current_year remains the mutable simulation state - Warn on init when regulated industries are absent from the model's industry set, with details on which codes were skipped - Update tests to reflect the baseline_emission_intensity rename
The post-2022 tightening formula (B - B * tightening_rate * (self.current_year - 2022) becomes negativewhere B = reduction_factor * self.baseline_emission_intensity[industry_idx]once tightening accumulates past the baseline, causing the allowable emission limit to go below zero (~2042 onward for a 2% annual rate). Clamp the returned limit to max(0, ...) so regulated sectors never receive a pathologically negative allowance. Add a test covering the far-future tightening case.
…tion This commit systematically addresses all critical and major review comments: **Critical Issues Fixed:** 1. Sparse-year price schedule crash: Replace index-based price array with year-based dictionary lookup to handle non-contiguous years (2014, 2019, 2030). Use forward-filling for intermediate years. 2. Year-boundary double-counting: Move reference accumulation window from 2017-2019 to 2017-2018, with limit computation in 2019. Prevents 2019 from being counted in both baseline and levy periods. 3. Missing reset state: Implement reset() method to clear all accumulators (reference_emission, reference_production, baseline_emission_intensity, emission_limit) for multi-run workflows. **Major Issues Fixed:** 4. CH4 field nullability: Add None guards in country.update_extra_taxes() when inputs_emissions_ch4 and capital_emissions_ch4 are not configured. Substitute zeros to prevent array addition errors. 5. Cost-push sign ambiguity: Include tax in unit costs calculation so positive OBPS levy correctly raises prices instead of suppressing them. Tax now shifts both numerator and denominator equally. 6. Price lookup bounds: Consolidated into new get_price() method that handles sparse schedules robustly via sorted year lookup. **Testing:** - Add comprehensive sparse-year integration tests matching real Canada OBPS schedule (2014, 2019, 2030) - Test forward-fill behavior for intermediate years - Test reset functionality for multi-run scenarios - Update existing tests to reflect 2017-2018 reference period - All 25 OBPS tests pass
…nterface consistency Ensures ExogenousPriceSetter.compute_price() has the same signature as other PriceSetter subclasses, preventing TypeError when OBPS combines with exogenous pricing setters.
7e9c4ad to
dec3d60
Compare
2 participants
feat: add Output-Based Pricing System (OBPS) (Example Canada Implementation)
Implements an industry-level carbon price signal based on each sector's
emissions relative to an output-based benchmark, as used in Canada's
federal OBPS regulation.
How it works
For each regulated sector i at time t (from 2019 onwards):
limit[i] = production[i] * reduction_factor[i] * reference_intensity[i]
obps_cost[i] = (input_emissions[i] + capital_emissions[i] - limit[i])
* carbon_price[t]
Note here that the cost can be negative: sectors emitting below their benchmark receive
a rebate that lowers their effective price, while sectors above the
benchmark face a positive marginal cost. Dividing by production gives a
per-unit marginal tax passed to price-setting and input-demand logic:
extra_marginal_taxes_firm[i] = obps_cost[i] / production[i]
The emission benchmark is output-based: the limit scales with production
so firms are not penalised for growing, only for exceeding the sector
intensity standard. Reference emission intensities are accumulated from
2017–2019 model periods. Post-2022 a tightening rate gradually lowers
the benchmark.
New components
macromodel/policy/output_based_price_system_can.py
OutputBasedPriceSystemCAN — computes sectoral OBPS cost each
period; accumulates 2017–2019 reference emissions to set
industry-specific intensity benchmarks.
macro_data/readers/policy_data/obps_can_reader.py
OBPSCANReader — reads industry reduction factors, tightening rates,
and the carbon price schedule; produces an OBPSCANData container.
Wiring
period when use_obps_reg is True; called from the planning phase so
the signal feeds into price-setting and input demand.
extra_marginal_taxes to shift the effective sector price.
Relevant tests have also been added.