-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CO2-eq units (plus design decisions) #9
Comments
@khaeru could you please comment on the approach in |
And I should note that, outside of design concerns, I am generally in favor a utilizing existing implementations when it makes sense especially if we can grow the user/dev community for specific tools. So in general, the more we can share between the IAM and SCM teams tool-wise, the better (many hands making lighter work and all..). |
I believe there is a larger issue than the technical implementation lurking in the shadows: @znicholls stop me if I'm completely wrong here, but afaik there is no unique value of a
So strictly speaking, it doesn't make sense to convert species without specifying the reference. I think that the behaviour of any implementation should be:
However, I don't see that implemented in If I'm correct, then we should use pint-contexts to choose the metric. |
Unfortunately we still had to more or less create a new units system in Pint. The
Spot on.
Hard-coded i.e. the user only has access to the metric conversions we've defined. The convention we've used so far is <IPCC report><metric><time horizon> e.g. SARGWP100. The metric conversions are loaded here (it's done like this so they can be lazy loaded i.e. we only read off disk when we need to).
I agree and it's how scmdata's unit registry behaves (see below). The keys are exactly what you've mentioned (I think):
>>> import scmdata
>>>
>>>
>>> scmdata.__version__
'0.4.0'
>>>
>>>
>>> ur = scmdata.units.unit_registry
>>>
>>>
>>> # 'standard' conversions are all fine
... ch4 = 1 * ur("Mt CH4 / yr")
>>> ch4.to("Gt CH4 / day")
<Quantity(2.737850787132101e-06, 'CH4 * gigametric_ton / day')>
>>>
>>>
>>> # if you try to convert CH4 to CO2 without a context, it fails
... try:
... ch4.to("Gt CO2 / yr")
... except DimensionalityError:
... traceback.print_exc(limit=0, chain=False)
...
Traceback (most recent call last):
pint.errors.DimensionalityError: Cannot convert from 'CH4 * megametric_ton / a' ([mass] * [methane] / [time]) to 'CO2 * gigametric_ton / a' ([carbon] * [mass] / [time])
>>> # with a valid context, it returns a result
... with ur.context("AR4GWP100"):
... ch4.to("Mt CO2 / yr")
...
<Quantity(25.0, 'CO2 * megametric_ton / a')>
>>> # the result depends on the metric
... with ur.context("AR5GWP100"):
... ch4.to("Mt CO2 / yr")
...
<Quantity(28.0, 'CO2 * megametric_ton / a')>
>>> with ur.context("SARGWP100"):
... ch4.to("Mt CO2 / yr")
...
<Quantity(21.0, 'CO2 * megametric_ton / a')>
>>>
>>> # simply 'GWP' is not a context
... try:
... with ur.context("GWP"):
... ch4.to("Mt CO2 / yr")
... except KeyError:
... traceback.print_exc(limit=0, chain=False)
...
Traceback (most recent call last):
KeyError: 'GWP'
>>>
>>> so2 = 1 * ur("Mt S / yr")
>>> so2.to("Mt SO2/yr")
<Quantity(2.0, 'SO2 * megametric_ton / a')>
>>>
>>> # if there is no valid conversion, a dimensionality error appears (context or not)
... try:
... so2.to("Gt CO2 / yr")
... except DimensionalityError:
... traceback.print_exc(limit=0, chain=False)
...
Traceback (most recent call last):
pint.errors.DimensionalityError: Cannot convert from 'S * megametric_ton / a' ([mass] * [sulfur] / [time]) to 'CO2 * gigametric_ton / a' ([carbon] * [mass] / [time])
>>> try:
... with ur.context("AR5GWP100"):
... so2.to("Mt CO2 / yr")
... except DimensionalityError:
... traceback.print_exc(limit=0, chain=False)
...
Traceback (most recent call last):
pint.errors.DimensionalityError: Cannot convert from 'S * megametric_ton / a' ([mass] * [sulfur] / [time]) to 'CO2 * megametric_ton / a' ([carbon] * [mass] / [time]) |
thanks for the clarification @znicholls! this level of sophistication wasn't clear to me from the source code and the docs... |
Our docs definitely leave something to be desired... |
Sorry if I'm missing something, but does #10 mean that the idea of a shared repository is off the table? |
I don't know, but I know we aimed for these light weight approaches in scmdata and abandoned them to avoid a) always reading off disk and b) problems related to compound units. This makes me nervous about the approach here (to be clear, the nerves don't mean it won't work, it's just not obvious to me that it will work yet). The reason I was suggesting a shared repository was to make it clear that we have all have some ownership (we'd all be contributors with the same access rights) and hence make clear that everyone's time would be worth investing. At the moment I have no stake in this project so it doesn't make sense for me to put time into it, especially given that we already have a fully tested solution. A shared repository might not suit you though, which is totally fine, I would just like a straight yes/no answer. |
Frankly, I don't see the need for a(nother) shared repo at this point. But I don't know how deep the rabbit hole goes and we might need a more elaborate solution in the future... About shared ownership - we migrated and renamed this repo partly because of your comment (as an extension of your suggestion |
Hi all - before we collectively decide to abandon a shared repo/resource (see prior reasoning why I do see it as a benefit), can I ask a few clarifying questions? Specifically for @znicholls:
|
Yes it would happen once and no I don't think it's enough to be a concern right now. My only hesitation is that if you want to do this for a lot of gases you could end up with rather big files (but I'd be happy to experiment with that).
There's a few little bugs that need hacks to get around. For example, Pint isn't that smart about dealing with spaces in units. So, for example, if you add the following test to #10 it will fail as shown. # new test
def test_conversion_tC_per_year(registry):
1 * registry("tC / yr") $ pytest test.py::test_conversion_tC_per_year -r a
=========================================================================================== test session starts ============================================================================================
platform darwin -- Python 3.7.4, pytest-5.3.5, py-1.8.1, pluggy-0.13.1
rootdir: /Users/znicholls/Documents/AGCEC/Misc/units
collected 1 item
test.py F [100%]
================================================================================================= FAILURES =================================================================================================
_______________________________________________________________________________________ test_conversion_tC_per_year ________________________________________________________________________________________
registry = <pint.registry.UnitRegistry object at 0x117f64410>
def test_conversion_tC_per_year(registry):
> 1 * registry("tC / yr")
test.py:92:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
...
> raise UndefinedUnitError(name_or_alias)
E pint.errors.UndefinedUnitError: 'tC' is not defined in the unit registry
venv/lib/python3.7/site-packages/pint/registry.py:626: UndefinedUnitError That means you have to define all the compound units (without spaces) explicitly or just never use units without spaces (which looks weird in plots). Most of the pint related code in scmdata is dealing with these sort of problems. So I guess what I was hoping is that we could start with what is in scmdata (given it circumvents most such issues) and then improve docs and simplify from there (so that the repo becomes maintainable by all). I'm happy to do that, I just want it to be of interest before I put in the couple of hours it'll take me to set things up.
Yes definitely
Yes, I think there'd be particular value in having more eyes rather than not (same as your initial comment). I think the value of combining the doc skills of this repo with the understanding of the units, gases and metrics available in scmdata (plus its handling of Pint features) would be of benefit to all. I'm also excited by the idea of working out how to integrate units with pandas a bit more (there is an effort here but it needs people to start trying to use it to see how it breaks). |
Ok, thanks @znicholls. Let's then see how this repo progresses and when, as there is interest and sufficient progress, we might be able to make that leap. I'll now close this issue as I think we've discussed as much as we can (others please feel free to reopen as needed). Just an aside on the units and spaces issue - a thought: What if we added a helper function in
Anyway, for a future discussion =) |
We were getting quite deep in the weeds in a conversation over #7. My suggestion is that we move that to an issue to hash out opinions that are not directly related to the implementation in the PR.
@khaeru opined (with example implementation here):
@znicholls identified and already implemented solution here
@danielhuppmann expressed some reservations regarding the heaviness implied on using that approach.
The conversation to be decided here is:
scmdata
, are we ok with derivative effects? (movingunits
to a package)The text was updated successfully, but these errors were encountered: