Repository navigation
Conversation
… into reduce_etas
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #476 +/- ##
==========================================
+ Coverage 68.94% 69.07% +0.12%
==========================================
Files 15 15
Lines 1755 1814 +59
==========================================
+ Hits 1210 1253 +43
- Misses 545 561 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
Hey,@C1587S @kemccusker I'm passing over this as I'm looking at wider updates and security. I wanted to give this PR a nudge. This has many changes and an increasing risk of going stale. This isn't marked as a draft PR. Is this ready to merge or work ongoing? Thanks! |
Member
C1587S
added a commit
that referenced
this pull request
Nov 13, 2026
…hies
damage_function_points, global_consumption_per_capita and
global_consumption branch on `self.geography != "globe"`. The "globe"
side is the 0.7.0 code: _damage_function_points_pandas is the previous
damage_function_points verbatim, and the two consumption methods keep
their previous expressions (`/ self.collapsed_pop.sum("region")`). It
returns what 0.7.0 returned: pd.DataFrame points and no region dimension
anywhere. The other side is PR #450's xarray code
(_damage_function_points_xarray) and returns an xr.Dataset with a region
dimension, which is what regional-scc step 1 reads.
PR #476's _aggregate_by_geography, _aggregate_to_country and
damages_dataset are lifted into MainRecipe as the single aggregation
implementation shared by Baseline, RiskAversionRecipe and EquityRecipe.
They replace PR #450's copy-pasted blocks in Baseline.damages_calculation,
RiskAversionRecipe.damages_calculation and
MainRecipe.global_consumption_per_capita (the fourth copy, the dead
collapsed_pop body, is removed two commits later). damages_dataset keeps
PR #450's individual_region handling: pop and damages are selected to
the region and the result has no region dimension. The regional
global_consumption now aggregates population at the same geography as
the per-capita series; PR #450 used IR-level population for "country", a
mismatch on a path nothing runs. Country mapping still comes from
`country_ISOs` / self.countries_mapping; the rename follows.
EquityRecipe gets the same treatment: its pandas path is untouched;
damages_dataset is overridden (the equity CE already collapses region, so
only "globe" is meaningful) and SUPPORTED_GEOGRAPHIES = ("globe",) makes
MainRecipe.__init__ raise NotImplementedError for Equity with "ir" or
"country" instead of silently broadcasting a region-less series over
regional population. Before this commit Equity with any geography failed
with AttributeError (no damages_calculation).
Left for later commits: the dead geography code (collapsed_pop body,
fit_project, geography_collapsed_econ_vars, the commented block in
baseline.py); the two discarded xr.where() masks on the xarray path (kept
verbatim and marked with a comment); country_ISOs; the adding_up eta
filename. ruff format is applied to baseline.py, main_recipe.py and
risk_aversion.py, which this commit rewrites; utils.py is formatted when
fit_project is removed.
Tests (Docker python:3.12 linux/amd64, the CI recipe): 225 passed /
3 skipped / 0 failed, the same as main. ruff check: 1 error (F821
dams_collapse); ruff format: utils.py only.
Legacy identity: 3 recipes x 7 discount types x {damage_function_points,
global_consumption, damage_function_coefficients, damage_function_fit,
uncollapsed_discount_factors, uncollapsed_marginal_damages, scc} =
168 outputs on the test fixtures, every one identical to main e900e00
(pandas assert_frame_equal with check_exact, xarray assert_identical),
in one local environment with numpy 1.26.4. 157 of 168 are also
byte-identical files; the other 11 are coefficient nc4 files whose
content is identical.
API checks, 54 of 54: default and geography="globe" give DataFrame points
and no region dims; "global" warns once from the caller's frame and
normalises; "ir" gives Dataset points with a region dimension whose sum
equals the globe damages; "ir" with individual_region gives region-less
consumption equal to the "ir" series at that region; Equity with "ir"
raises NotImplementedError; an unknown geography raises ValueError;
StackedDamages alone defaults to "globe".
C1587S
added a commit
that referenced
this pull request
Nov 13, 2026
PR #450 computed the SSP1/rcp85, SSP5/rcp45 and agriculture ACCESS1-0/rcp85 masks in damage_function_points but discarded the xr.where() results, so on the regional path those combinations entered the damage function fit. _filter_illegal_combinations_xr (from PR #476, with two guards: a 0-d ssp coordinate under gwr discounting, and a missing rcp coordinate for slr sectors) now sets anomaly/gmsl to NaN for those combinations, exactly what the pandas path does with .loc. damages is never masked, on either path. The pandas path is untouched. What it changes (test fixtures with all five SSPs, geography="ir", Baseline and RiskAversionRecipe): before: 97,416 of 487,080 anomaly cells belong to illegal combinations; 95,940 of them were finite and went into the fit after: 0 finite; the xarray path run at "globe" now matches the pandas path row for row (54,120 rows, damages equal, NaN pattern equal, 11,480 masked rows on each side) Shift against the 2026-11-12 regional-scc baseline: zero. Its step-1 points hold ssp=["SSP2"] only, the coastal_v0.22 sector has an slr coordinate and no rcp, and the sector name does not contain "agriculture", so every mask condition is false there, before and after. Step 3 loads coefficients from disk and never builds points. A regional-scc run that includes SSP1 or SSP5, or an agriculture sector, will change: those combinations are dropped from the fit, as they always were on the global path. Tests (Docker, CI recipe): 225 passed / 3 skipped / 0 failed.
C1587S
added a commit
that referenced
this pull request
Nov 13, 2026
tests/test_geography.py and the menu_instance_with_geography fixture come from PR #476's branch, the only geography regression tests that existed anywhere; they now cover the code the preceding commits rewrote. Adapted to the current API: `country_mapping_path` (PR #476's name); the xarray path returns an xr.Dataset, so TestDualPathEquivalence converts at the test boundary; the country-mapping error names `country_mapping_path`. TestIndividualRegion is un-skipped and TestCountryAggregation has a real mapping fixture instead of a skip. Added the warning tests: the "global" DeprecationWarning, the country_ISOs DeprecationWarning and both legacy-store UserWarnings (legacy store read on the globe and regional paths; the write side is in test_preprocessing) fire exactly once where intended, point at the caller's frame, and do not fire on the default construction, on `geography="globe"`, on `country_mapping_path`, or when the eta-suffixed stores exist. The one-off API checks run on the preceding commits are folded in so the repository suite carries them: default and "globe" give DataFrame points with no region dimension on consumption or discount factors; "ir" gives Dataset points on the nine fixture regions whose sum equals the globe damages; the xarray path at "globe" with all five SSPs matches the pandas path row for row including the NaN pattern of the illegal-combination mask; "ir" with all five SSPs has no finite anomaly at SSP1/rcp85 or SSP5/rcp45 and nothing masked with SSP2-4 (the regional-scc configuration); "country" sums to the globe damages with an unmapped ISO in "nopop"; EquityRecipe rejects "ir" and "country"; individual_region reproduces that slice of the all-region run for damages and consumption, and calculated_damages is an unweighted property. Fixtures in conftest.py: geography_menu_factory, country_mapping_csv, legacy_ce_paths, GEOGRAPHY_REGIONS, INDIVIDUAL_REGION. No behaviour change: tests only. Tests (Docker, CI recipe): 291 passed / 3 skipped / 0 failed (225 + 66).
C1587S
added a commit
that referenced
this pull request
Nov 13, 2026
Rewrites the Unreleased section so a reader who never opens git log can see who wrote what. PR #417's entries (@JMGilbert) are kept verbatim. Added: PR #450's per-region damage functions, individual_region and Baseline.calculated_damages (@JMGilbert); country_mapping_path, the shared aggregation helpers, damages_dataset and tests/test_geography.py (PR #476, @C1587S); the normalisation and store helpers. Changed: Climate.gmsl returning the whole store (PR #450), geography normalisation and the Equity restriction, the legacy adding_up store name and two-sided fallback with its stated residual behaviour. Deprecated: geography="global" and country_ISOs. Fixed: PR #450's own extrapolation, regional-SCC and double-counted-population fixes, the illegal SSP/RCP masking on the regional path (PR #476's implementation), preds restored, the pandas-on-xarray and Equity breakages of the merged tree. Removed: PR #417's adding_up eta assertion and, stated explicitly as asked, the "quantile regression is not compatible with risk aversion" assertion that PR #417 dropped from reduce_damages(); the dead PR #450 code; the countries_mapping/countries attributes. Notes for users: h5py is not declared and conda users must add it; outputs carry no region dimension under the default geography, so scripts written against PR #450's branch must drop .sel(region="globe") on uncollapsed_discount_factors. Documentation only; no code change.
C1587S
added a commit
that referenced
this pull request
Nov 13, 2026
Re-expresses the one remaining hunk of an earlier individual_region patch of mine, written against PR #476's branch, and records what became of the rest. Dropped because PR #450 or the dual-path commit already does it: - baseline.py: `calculated_damages` as an unweighted property reading the eta-suffixed adding_up stores: PR #450 (27f0a91, 549e0fe), kept by the dual-path commit, which folded in its docstring. - baseline.py: `global_consumption_calculation` selecting gdp for the region: the same series is reached through geography="ir" plus the `.sel(region=individual_region)` in `global_consumption`. - main_recipe.py: `damages_dataset` selecting damages and pop to the region: present in `damages_dataset`, individual_region branch. - main_recipe.py: `global_consumption_per_capita` dividing by the region's population: the dual path divides by population at the same geography and `global_consumption` selects the region. - main_recipe.py: `global_consumption` 2300 and extrapolation branches and the `betas.sel(region=...)` in global_consumption_no_pulse and global_consumption_pulse: PR #450 (27f0a91, c83829e), kept. - tests/conftest.py `ir_menu_factory`: the ported geography_menu_factory. - tests/test_geography.py TestIndividualRegion: five of the seven tests were ported with the geography tests (no region dim; matches the slice; differs from the other regions; consumption follows the region; calculated_damages is an unweighted property). Dropped because it would remove something that works: - `NotImplementedError` for individual_region on non-adding_up recipes. RiskAversionRecipe with geography="ir" and individual_region reproduces that slice of the all-region run for damages and consumption under euler_ramsey, constant_model_collapsed and gwr_gwr; the test here asserts that instead of forbidding it. (EquityRecipe is already limited to "globe".) Re-expressed: - the patch raised ValueError for individual_region with geography="globe", where summed global damages would be paired with single-region consumption. On the merged tree that combination raised an opaque KeyError inside global_consumption; on dscim <= 0.7.0 the keyword fell into **kwargs and was ignored. Since the library must not raise at a caller the released version accepts, MainRecipe.__init__ now ignores individual_region when geography is "globe", emits a UserWarning naming the keyword, the default and what to pass instead (geography="ir"), and produces exactly the 0.7.0 result. Tests: the warning fires once, from the caller, and not under "ir"; points and consumption equal the plain globe run. Found on the way, recorded as a strict xfail, not fixed here: Baseline with any gwr discounting on the regional path fails in damages_dataset ("dimension 'ssp' already exists as a scalar variable") because the adding-up stores keep ssp/model dimensions that the pooled scalar coordinates collide with. Pre-existing on PR #450's branch (where DataArray.assign raised AttributeError instead) and on PR #476's; nothing runs that combination (regional-scc uses euler_ramsey); RiskAversionRecipe collapses ssp/model in its CE and works. Tests: tests/test_geography.py 72 passed, 1 xfailed.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds geography-level aggregation support to
MainRecipe, enabling damage calculations at different regional levels while maintaining backward compatibilityChanges
New parameters in
MainRecipe.__init__:geography: One of "ir" (impact region), "country", or "globe" (default)country_mapping_path: Path to CSV mapping regions to countries (required for country aggregation)individual_region: Optional filter for single-region calculationsNew methods:
_aggregate_by_geography(): Internal helper for geography aggregation_aggregate_to_country(): Helper for country-level aggregationdamages_dataset(): Returns damages asxr.Datasetwith geography support_filter_illegal_combinations_xr(): Xarray version of illegal combination filtering_damage_function_points_pandas(): Original pandas-based implementation_damage_function_points_xarray(): New xarray-based implementationDual-path execution:
geography="globe"(default): uses original pandas pathgeographyis "ir" or "country": uses new xarray pathpd.DataFrameat the API boundaryBackward compatibility
geography="globe"preserves existing behaviorglobal_damages_calculation()returnspd.DataFrame(unchanged)damage_function_pointsreturnspd.DataFrame(unchanged)Related PRs
Current status