From ad2f7d3d34c09d4089c4c03ebf7ea3c5bed9f7be Mon Sep 17 00:00:00 2001 From: Hugh Runyan Date: Tue, 22 Sep 2026 11:47:03 -0700 Subject: [PATCH] The gas-capture flag gates the site-type default in both trace paths `site_only_estimate_trace` and `citysite_estimate_trace` carried copies of the gas-capture logic and the copies drifted. The citysite copy applied the site-type default (0.60 / 0.45 / 0.0) with no check on `landfill_gas_collection`, while still reading that same flag to pick `ox_cap` vs `ox_nocap`. So a city-linked landfill was built with `gas_capture=False` and `gas_capture_efficiency=0.6` on the same object. On the 10_05_26 submission that hit 1,994 sites: every Brazilian city-linked asset and nothing else. 959 controlled dumpsites at 0.45 and 1,035 sanitary landfills at 0.60, all flagged False. Mexico's 1,938 equally-unverified False sites ran at zero, because they take the site_only path. Same history as the MCF table, same fix. `_resolve_gas_capture` is now the single definition of the rule and both methods call it; `GAS_EFF_OPTIONS` and `OX_OPTIONS` move to module level and the two per-method copies are deleted. `site_only`'s semantics are preserved exactly. citysite additionally picks up the pd.NA guard (its `== True` raised "boolean value of NA is ambiguous" on a null flag) and the empty-gascap_df fallback. The copies in `sinar_city_and_site` and `site_only_estimate` are untouched -- neither is on the Climate TRACE path. Model output moves: Brazil's modelled FOD rises ~2.34x on those 1,994 sites, about +879 kt CH4 in 2024. Their published uncertainty also rises, from 0.254 to ~0.446, because `uncertainty.py` already assumed the gated behaviour. Co-Authored-By: Claude Opus 5 --- SWEET_python/city_params.py | 232 +++++++++++++++------------------ tests/test_gas_capture_gate.py | 164 +++++++++++++++++++++++ 2 files changed, 271 insertions(+), 125 deletions(-) create mode 100644 tests/test_gas_capture_gate.py diff --git a/SWEET_python/city_params.py b/SWEET_python/city_params.py index 489f0bc..5c0b7a2 100644 --- a/SWEET_python/city_params.py +++ b/SWEET_python/city_params.py @@ -106,6 +106,103 @@ def _build_oxidation_series(default_value, canonical_row, time_series_rows, year return series +# Site-type fallbacks for gas capture and oxidation. Single definition, shared by both +# `_trace` entry points via `_resolve_gas_capture` below. `uncertainty.py` in the TRACE +# pipeline mirrors these tables (SWEET_GAS_CAPTURE_DEFAULTS / +# SWEET_OXIDATION_DEFAULTS_*), so a change here must be made there too. +GAS_EFF_OPTIONS = { + "Sanitary Landfill": 0.6, + "Controlled Dumpsite": 0.45, + "Dumpsite": 0.0, +} +OX_OPTIONS = { + "ox_nocap": {"Sanitary Landfill": 0.1, "Controlled Dumpsite": 0.05, "Dumpsite": 0.0}, + "ox_cap": {"Sanitary Landfill": 0.22, "Controlled Dumpsite": 0.1, "Dumpsite": 0.0}, +} + + +def _resolve_gas_capture(flag, site_type, canonical_row, time_series_rows, years_range): + """Gas-capture presence, oxidation default and per-year capture efficiency. + + THE RULE, which is the whole point of this function existing: + **the boolean GATES the site-type default.** A measured + ``gas_collection_efficiency`` wins over both; absent one, the type default + (:data:`GAS_EFF_OPTIONS`) applies ONLY when presence is true, and presence also + selects ``ox_cap`` vs ``ox_nocap``. A site with no recorded system is modelled at + zero capture, and an unknown flag counts as no system -- the same "unknown means + none" convention as the TRACE pipeline's ``mitigation_selector`` and + ``uncertainty.recovery_and_oxidation_components``. + + WHY IT IS A FUNCTION. This logic was copied into ``site_only_estimate_trace`` and + ``citysite_estimate_trace`` and the copies drifted: the citysite copy applied the + type default unconditionally while still reading the flag for oxidation, so a + Brazilian city-linked landfill was built with ``gas_capture=False`` and + ``gas_capture_efficiency=0.6`` on the same object. That was 1,994 sites on the + 10_05_26 submission -- every city-linked asset, and nothing else. Same history as + the MCF table, same fix: one definition, two callers. + + Returns ``(presence, oxidation_value, gas_capture_efficiency)`` where presence is a + plain ``bool``, oxidation_value is the type default for that presence (the caller + may still override it per-year via :func:`_build_oxidation_series`), and + gas_capture_efficiency is a Series over ``years_range``. + """ + # Presence. `pd.NA` is the reason this is not a bare truthiness test: `pd.NA == True` + # is `pd.NA`, and `bool(pd.NA)` raises "boolean value of NA is ambiguous". Check + # isna FIRST, before any comparison. + if pd.isna(flag): + presence = False + elif (flag == "Yes") or (flag is True) or (flag == True): # noqa: E712 + presence = True + else: + # Numeric or string-ish flags from older inputs: >0 means a system. + try: + presence = bool(flag == flag and flag > 0) + except Exception: + presence = False + + oxidation_value = OX_OPTIONS["ox_cap" if presence else "ox_nocap"][site_type] + + # Measured per-year capture, where the site has it. Keyed on + # `reported_emissions_year`, so a row without one cannot be placed. Drop on BOTH + # columns: a row can carry a reported year with no capture value, or a capture + # value with no year. + # + # The all-dropped case is reachable and is not an edge case. The TRACE input query + # selects gas_collection_efficiency independently of CH4_reported, and + # reported_emissions_year is DERIVED from CH4_reported + # (landfill_table_ops.py: extract_emissions_year_from_dict). A site with capture + # data but no reported emissions therefore has no reported year at all -- and, + # because a null CH4_reported is exactly what routes a site to 'to be modeled', + # such a site reaches this branch rather than the reported pathway. Taking .mean() + # of the emptied frame yielded NaN and poisoned the whole capture series; fall back + # to the gated default instead, matching the scalar path below. + gascap_df = None + if isinstance(time_series_rows, pd.DataFrame): + if time_series_rows['gas_collection_efficiency'].notna().any(): + gascap_df = time_series_rows[['reported_emissions_year', 'gas_collection_efficiency']] + gascap_df = gascap_df.dropna( + subset=['reported_emissions_year', 'gas_collection_efficiency'] + ).copy() + if gascap_df.empty: + gascap_df = None + + if gascap_df is not None: + mean = gascap_df['gas_collection_efficiency'].mean() + gas_capture_efficiency = pd.Series(mean, index=years_range) + gas_capture_efficiency.loc[gascap_df['reported_emissions_year'].values] = ( + gascap_df['gas_collection_efficiency'].values + ) + return presence, oxidation_value, gas_capture_efficiency + + # Single-row sites carry any measured value on the canonical row instead. + value = canonical_row['gas_collection_efficiency'] if not isinstance( + time_series_rows, pd.DataFrame + ) else np.nan + if pd.isna(value): + value = GAS_EFF_OPTIONS[site_type] if presence else 0 + return presence, oxidation_value, pd.Series(value, index=years_range) + + # The way this model is set up is based on the unit of a City, corresponding to the City class. # Cities can have multiple sets of CityParameters, one for each scenario. # Sets of CityParameters can have one or more landfills, dumpsites, waste to energy, etc. @@ -2467,23 +2564,6 @@ def site_only_estimate_trace(self, canonical_row=None, time_series_rows=None, po "Controlled Dumpsite": 1, "Dumpsite": 2, } - ox_options = { - "ox_nocap": { - "Sanitary Landfill": 0.1, - "Controlled Dumpsite": 0.05, - "Dumpsite": 0.0, - }, - "ox_cap": { - "Sanitary Landfill": 0.22, - "Controlled Dumpsite": 0.1, - "Dumpsite": 0.0, - }, - } - gas_eff_options = { - "Sanitary Landfill": 0.6, - "Controlled Dumpsite": 0.45, - "Dumpsite": 0.0, - } # Get the most common non-NaN value, or 3 if all are NaN depth = canonical_row['waste_depth'] site_type = canonical_row['type'] @@ -2512,72 +2592,10 @@ def site_only_estimate_trace(self, canonical_row=None, time_series_rows=None, po else: gas_capture_presence = canonical_row['other7'] - # Handle pd.NA / missing: avoid "boolean value of NA is ambiguous" in comparisons - if pd.isna(gas_capture_presence): - gas_capture_presence = False - oxidation_value = ox_options["ox_nocap"][site_type] - elif (gas_capture_presence == "Yes") or (gas_capture_presence is True) or (gas_capture_presence == True): - gas_capture_presence = True - oxidation_value = ox_options["ox_cap"][site_type] - else: - try: - if gas_capture_presence == gas_capture_presence: - if gas_capture_presence > 0: - gas_capture_presence = True - oxidation_value = ox_options["ox_cap"][site_type] - else: - gas_capture_presence = False - oxidation_value = ox_options["ox_nocap"][site_type] - else: - gas_capture_presence = False - oxidation_value = ox_options["ox_nocap"][site_type] - except: - gas_capture_presence = False - oxidation_value = ox_options["ox_nocap"][site_type] - - if isinstance(time_series_rows, pd.DataFrame): - gascap_df = None - if time_series_rows['gas_collection_efficiency'].notna().any(): - # Measured per-year capture is keyed on reported_emissions_year, so a row - # without one cannot be placed. Drop on BOTH columns: a row can carry a - # reported year with no capture value, or a capture value with no year. - # - # The all-dropped case is reachable and is not an edge case. The TRACE - # input query selects gas_collection_efficiency independently of - # CH4_reported, and reported_emissions_year is DERIVED from CH4_reported - # (landfill_table_ops.py: extract_emissions_year_from_dict). A site with - # capture data but no reported emissions therefore has no reported year at - # all -- and, because a null CH4_reported is exactly what routes a site to - # 'to be modeled', such a site reaches this branch rather than the reported - # pathway. Taking .mean() of the emptied frame yielded NaN and poisoned the - # whole capture series; fall back to the site-type default instead, matching - # the scalar path below. - gascap_df = time_series_rows[['reported_emissions_year', 'gas_collection_efficiency']] - gascap_df = gascap_df.dropna( - subset=['reported_emissions_year', 'gas_collection_efficiency'] - ).copy() - if gascap_df.empty: - gascap_df = None - - if gascap_df is not None: - gas_capture_efficiency_mean = gascap_df['gas_collection_efficiency'].mean() - gas_capture_efficiency = pd.Series(gas_capture_efficiency_mean, index=self.years_range) - gas_capture_efficiency.loc[gascap_df['reported_emissions_year'].values] = gascap_df['gas_collection_efficiency'].values - else: - if gas_capture_presence is True: - gas_capture_efficiency = gas_eff_options[site_type] - else: - gas_capture_efficiency = 0 - gas_capture_efficiency = pd.Series(gas_capture_efficiency, index=self.years_range) - else: - gas_capture_efficiency = canonical_row['gas_collection_efficiency'] - if pd.isna(gas_capture_efficiency): - if gas_capture_presence is True: - gas_capture_efficiency = gas_eff_options[site_type] - else: - gas_capture_efficiency = 0 - gas_capture_efficiency = pd.Series(gas_capture_efficiency, index=self.years_range) - + gas_capture_presence, oxidation_value, gas_capture_efficiency = _resolve_gas_capture( + gas_capture_presence, site_type, canonical_row, time_series_rows, self.years_range + ) + mcf = mcf_defaults.mcf_for_site(site_type_idx, depth) open_date = canonical_row['site_open_year'] if isinstance(open_date, str): @@ -2783,23 +2801,6 @@ def citysite_estimate_trace(self, canonical_row=None, time_series_rows=None, cit "Controlled Dumpsite": 1, "Dumpsite": 2, } - ox_options = { - "ox_nocap": { - "Sanitary Landfill": 0.1, - "Controlled Dumpsite": 0.05, - "Dumpsite": 0.0, - }, - "ox_cap": { - "Sanitary Landfill": 0.22, - "Controlled Dumpsite": 0.1, - "Dumpsite": 0.0, - }, - } - gas_eff_options = { - "Sanitary Landfill": 0.6, - "Controlled Dumpsite": 0.45, - "Dumpsite": 0.0, - } # Get the most common non-NaN value, or 3 if all are NaN depth = canonical_row['waste_depth'] site_type = canonical_row['type'] @@ -2828,32 +2829,13 @@ def citysite_estimate_trace(self, canonical_row=None, time_series_rows=None, cit else: gas_capture_presence = canonical_row['other7'] - if gas_capture_presence == "Yes" or gas_capture_presence == True: - gas_capture_presence = True - oxidation_value = ox_options["ox_cap"][site_type] - else: - gas_capture_presence = False - oxidation_value = ox_options["ox_nocap"][site_type] - - if isinstance(time_series_rows, pd.DataFrame): - if time_series_rows['gas_collection_efficiency'].notna().any(): - gascap_df = time_series_rows[['reported_emissions_year', 'gas_collection_efficiency']] - gascap_df = gascap_df.dropna(subset=['reported_emissions_year']).copy() - gas_capture_efficiency_mean = gascap_df['gas_collection_efficiency'].mean() - gas_capture_efficiency = pd.Series(gas_capture_efficiency_mean, index=self.years_range) - gas_capture_efficiency.loc[gascap_df['reported_emissions_year'].values] = gascap_df['gas_collection_efficiency'].values - else: - gas_capture_efficiency = canonical_row['gas_collection_efficiency'] - if pd.isna(gas_capture_efficiency): - gas_capture_efficiency = gas_eff_options[site_type] - gas_capture_efficiency = pd.Series(gas_capture_efficiency, index=self.years_range) + # Was a drifted copy of the block above: it applied the site-type default + # WITHOUT checking the flag, while still reading the flag for oxidation. Now + # both paths share `_resolve_gas_capture`, which is where the rule lives. + gas_capture_presence, oxidation_value, gas_capture_efficiency = _resolve_gas_capture( + gas_capture_presence, site_type, canonical_row, time_series_rows, self.years_range + ) - else: - gas_capture_efficiency = canonical_row['gas_collection_efficiency'] - if pd.isna(gas_capture_efficiency): - gas_capture_efficiency = gas_eff_options[site_type] - gas_capture_efficiency = pd.Series(gas_capture_efficiency, index=self.years_range) - mcf = mcf_defaults.mcf_for_site(site_type_idx, depth) open_date = canonical_row['site_open_year'] if isinstance(open_date, str): diff --git a/tests/test_gas_capture_gate.py b/tests/test_gas_capture_gate.py new file mode 100644 index 0000000..99bf01a --- /dev/null +++ b/tests/test_gas_capture_gate.py @@ -0,0 +1,164 @@ +"""The gas-capture boolean gates the site-type default -- in BOTH trace paths. + +`site_only_estimate_trace` and `citysite_estimate_trace` used to carry copies of this +logic and the copies drifted: the citysite copy applied the site-type default with no +check on the flag, while still reading that same flag to pick oxidation. On the +10_05_26 submission that gave 1,994 Brazilian city-linked sites a 0.60/0.45 capture +efficiency they had no recorded system for -- and a Landfill object carrying +`gas_capture=False` next to `gas_capture_efficiency=0.6`. + +Same history as the MCF table (see test_mcf.py), same fix: one definition, two callers. +These tests exercise the shared resolver directly, so they pin the rule without needing +a live City run. +""" + +import numpy as np +import pandas as pd +import pytest + +from SWEET_python.city_params import ( + GAS_EFF_OPTIONS, + OX_OPTIONS, + _resolve_gas_capture, +) + + +YEARS = range(1970, 2051) +TYPES = ["Sanitary Landfill", "Controlled Dumpsite", "Dumpsite"] + + +def _canonical(gce=np.nan): + return pd.Series({"gas_collection_efficiency": gce}) + + +def _rows(pairs): + """Multi-row site frame: [(reported_emissions_year, gas_collection_efficiency), ...]""" + return pd.DataFrame( + pairs, columns=["reported_emissions_year", "gas_collection_efficiency"] + ) + + +# --------------------------------------------------------------------------- # +# The rule itself +# --------------------------------------------------------------------------- # + +@pytest.mark.parametrize("site_type", TYPES) +@pytest.mark.parametrize("flag", [False, None, np.nan, pd.NA, 0, "No"]) +def test_no_recorded_system_means_zero_capture(site_type, flag): + """A flag that is not a yes gets zero capture, never the site-type default. + + This is the assertion the citysite copy failed. `pd.NA` is in the list because + the un-hardened copy raised "boolean value of NA is ambiguous" on it. + """ + presence, ox, gce = _resolve_gas_capture( + flag, site_type, _canonical(), _rows([]), YEARS + ) + assert presence is False + assert (gce == 0).all() + assert ox == OX_OPTIONS["ox_nocap"][site_type] + + +@pytest.mark.parametrize("site_type", TYPES) +@pytest.mark.parametrize("flag", [True, "Yes", 1]) +def test_recorded_system_gets_the_site_type_default(site_type, flag): + presence, ox, gce = _resolve_gas_capture( + flag, site_type, _canonical(), _rows([]), YEARS + ) + assert presence is True + assert (gce == GAS_EFF_OPTIONS[site_type]).all() + assert ox == OX_OPTIONS["ox_cap"][site_type] + + +def test_a_flagged_dumpsite_still_captures_nothing(): + # Dumpsite's default IS zero, so "flag true, capture zero" is correct here and + # must not be read as the gate failing. + presence, _, gce = _resolve_gas_capture(True, "Dumpsite", _canonical(), _rows([]), YEARS) + assert presence is True + assert (gce == 0).all() + + +# --------------------------------------------------------------------------- # +# Measured values override the gate +# --------------------------------------------------------------------------- # + +def test_measured_efficiency_overrides_a_false_flag(): + """GHGRP measures gas being collected at 87 sites LMOP flags as having no system. + The measurement wins -- see build_gas_capture_rates.classify_assets in the TRACE + repo, which documents the same precedence. + """ + rows = _rows([(2020, 0.35), (2021, 0.45)]) + presence, _, gce = _resolve_gas_capture(False, "Sanitary Landfill", _canonical(), rows, YEARS) + assert presence is False # the flag is still reported honestly + assert gce.loc[2020] == 0.35 # but the measurement drives the model + assert gce.loc[2021] == 0.45 + assert gce.loc[1999] == pytest.approx(0.40) # off-year baseline = site mean + + +def test_scalar_measured_value_on_a_single_row_site(): + presence, _, gce = _resolve_gas_capture( + False, "Sanitary Landfill", _canonical(gce=0.72), None, YEARS + ) + assert presence is False + assert (gce == 0.72).all() + + +# --------------------------------------------------------------------------- # +# The NaN-poisoning trap +# --------------------------------------------------------------------------- # + +@pytest.mark.parametrize("flag,expected", [(True, 0.6), (False, 0.0)]) +def test_capture_without_a_reported_year_falls_back_to_the_gate(flag, expected): + """A site can carry capture data and no reported emissions year at all -- the + input query selects the two independently, and a null CH4_reported is exactly what + routes a site to 'to be modeled'. Dropping every row once left .mean() as NaN and + poisoned the whole series. It must fall back to the GATED default, not to NaN and + not to the default unconditionally. + """ + rows = _rows([(np.nan, 0.5)]) + _, _, gce = _resolve_gas_capture(flag, "Sanitary Landfill", _canonical(), rows, YEARS) + assert gce.notna().all() + assert (gce == expected).all() + + +def test_a_year_without_a_capture_value_is_dropped_not_averaged_as_nan(): + rows = _rows([(2020, 0.5), (2021, np.nan)]) + _, _, gce = _resolve_gas_capture(True, "Sanitary Landfill", _canonical(), rows, YEARS) + assert gce.notna().all() + assert gce.loc[2020] == 0.5 + assert gce.loc[2021] == 0.5 # baseline = mean of the one usable row + + +# --------------------------------------------------------------------------- # +# Both callers go through the resolver +# --------------------------------------------------------------------------- # + +def test_neither_trace_path_keeps_its_own_copy_of_the_tables(): + """Guards the refactor: if someone re-inlines the tables into either `_trace` + method, the drift can start again. The two surviving `gas_eff_options` locals + belong to `sinar_city_and_site` and `site_only_estimate`, which are not the + Climate TRACE pipeline path. + """ + import inspect + + from SWEET_python.city_params import City + + for method in (City.site_only_estimate_trace, City.citysite_estimate_trace): + src = inspect.getsource(method) + assert "gas_eff_options" not in src, f"{method.__name__} re-inlined the table" + assert "ox_options" not in src, f"{method.__name__} re-inlined the table" + assert "_resolve_gas_capture" in src, f"{method.__name__} bypasses the resolver" + + +def test_oxidation_and_capture_never_disagree_about_presence(): + """The bug's signature: oxidation said no-capture while capture said 0.6. No input + may produce that combination again. + """ + for site_type in TYPES: + for flag in [True, False, None, np.nan, pd.NA, "Yes", "No", 0, 1]: + presence, ox, gce = _resolve_gas_capture( + flag, site_type, _canonical(), _rows([]), YEARS + ) + expected_ox = OX_OPTIONS["ox_cap" if presence else "ox_nocap"][site_type] + assert ox == expected_ox, (site_type, flag) + if not presence: + assert (gce == 0).all(), (site_type, flag)