Fix TypeError for pGWP/pGTP with IPCC fallback functions - #29
Merged
Merged
Conversation
The pGWP and pGTP metrics passed `time_varying_re` to every flow's
characterization function. With `fallback_to_ipcc=True` (the default), flows
that are not covered by the Watanabe module - CO and the GHGs built from
decay_multipliers.json - are characterized with IPCC AR6 functions, whose
signature is `(series, period, cumulative)`. Characterizing such a flow raised
TypeError: create_generic_characterization_function.<locals>.characterize_generic()
got an unexpected keyword argument 'time_varying_re'
`_characterize_prospective_radiative_forcing` already guarded against this
inline. That check is now a shared helper, `_apply_characterization_function`,
used by the pGWP, pGTP and prospective radiative forcing paths alike, so the
argument only reaches functions that accept it.
Prospective metrics are typically calculated through another package (bw_timex), so users hitting this error have never imported dynamic_characterization themselves. "Call prospective.set_scenario(iam, ssp, rcp) first" then leaves them guessing what `prospective` is and which arguments are valid. The message now states which metrics need a scenario, gives a copy-pasteable call with the full import path, says that it is set once per session, and lists the IAM-SSP pairs plus a pointer to VALID_SCENARIOS.
Member
Author
|
Pushed a second, related usability fix onto this branch: the Old message: That assumes the reader knows what Covered by |
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.
Problem
Calculating
pGWP(orpGTP) on an inventory that contains a GHG outside the Watanabe module fails:_characterize_pgwppassedtime_varying_reto every flow's characterization function. Withfallback_to_ipcc=True(the default), flows that Watanabe doesn't cover - CO and the GHGs built fromdecay_multipliers.json- are mapped to IPCC AR6 functions with signature(series, period, cumulative), which have no such parameter. In practice this hits any ecoinvent-basedpGWPcalculation, since the usual climate change methods characterize more than CO2/CH4/N2O.Fix
_characterize_prospective_radiative_forcingalready had this check inline. It is now the shared helper_apply_characterization_function, which passestime_varying_reonly to the Watanabe functions and is used by the pGWP, pGTP and prospective radiative forcing paths alike.Tests
New
tests/test_characterize_prospective_fallback.py: a mixed characterization function dict (prospective CH4 + IPCCcharacterize_co+ a generic decay-series function) across the metricspGWP,pGTPandprospective_radiative_forcing, withtime_varying_rebothFalseandTrue. It reproduces theTypeErrorwithout the fix and asserts that all three flows end up characterized. Full suite: 100 passed.Note on the file name:
test_prospective.pyreplacesdynamic_characterizationinsys.moduleswith a stub at import time, so any test module importing the real package has to sort before it alphabetically. That is noted in the new module's docstring, but it is a landmine worth removing separately.Not addressed here
For IPCC fallback gases,
pGWPdivides an AR6 AGWP by a Watanabe AGWP for CO2, i.e. a mixed reference. Whether these gases should instead use the IPCC CO2 reference is a modelling decision, left out of this fix.