Add units

[znicholls]

notebooks/emissions-units-with-pint.ipynb gives an example of how this works with Pint

[codecov]

Codecov Report

Merging #35 into master will increase coverage by 18.1%.
The diff coverage is 96.49%.

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   59.01%   77.11%   +18.1%     
==========================================
  Files           5        6       +1     
  Lines          61      118      +57     
  Branches        0       11      +11     
==========================================
+ Hits           36       91      +55     
  Misses         25       25              
- Partials        0        2       +2

Impacted Files	Coverage Δ
openscm/units.py	96.49% <96.49%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38ad4ba...2f058b4. Read the comment docs.

[rgieseke]

Haha, before i can even comment the CEDS stuff is gone :-)

[rgieseke]

To discuss:
Hyphens or Underscores in unit names:

Maybe "N2O_N"

N2O-N is not possible I think, they need to be valid Python variable names.

[znicholls]

My vote would be neither. Removed ambiguity about whether an underscore is really an underscore or whether it's meant to be a hyphen. Option b is to have a convenience function for conversion which understands hyphens, strips them, gets the conversions and then returns the results with hyphens back in.

…

On Tue, 30 Oct 2018 at 11:31 am, Robert Gieseke ***@***.***> wrote: To discuss: Hyphens or Underscores in unit names: Maybe "N2O_N" N2O-N is not possible I think, they need to be valid Python variable names. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWh-m0ms3Ov_CgEAbfkMz1lE_1mIq95Zks5uqCpogaJpZM4YAUT6> .

[swillner]

In general, are the units we would actually use so complex we need a sophisticated library for that?

[swillner]

Hmmm, having looked into it, I am not that happy with using Pint. If I have two units and want to do lots of conversions between the two, I cannot get just a (pre-/post-) offset and a scaling factor (without hacking Pint or resorting to calculating these myself from some chosen points). Anything else would be inefficient, at least in the low-level API... What can we do about that?
Where do you need conversion in the high-level API?
What units do we need actually?

[swillner]

Alternatively, we could have a look at unyt, which seems to have the functionality I want: https://unyt.readthedocs.io/en/latest/modules/unyt.unit_object.html#unyt.unit_object.Unit.get_conversion_factor

[rgieseke]

@swillner What unit conversion is happening in the low-level API?

[znicholls]

In general, are the units we would actually use so complex we need a sophisticated library for that?

I would argue yes. People (me) have cocked them up enough to mean that if we can just do the unit handling once, in a fairly clever way, we should.

[znicholls]

Hmmm, having looked into it, I am not that happy with using Pint.

It's the only unit library with Pandas support. For me that's reason enough to use it.

What can we do about that?

I'm happy to make a PR to Pint this week which gives you a convenience function to Pint which does this (or put it first in OpenSCM, then make a PR in Pint)

>>> import pint
>>> ureg = pint.UnitRegistry()
>>> a = 1*ureg.m
>>> a
<Quantity(1, 'meter')>
>>> a.to("km").magnitude
0.001

It would look like this

def get_conversion_factor(start_units, other_units):
    # type checking etc goes here
    pint_quantity = 1 * ureg(start_units)
    return pint_quantity.to(other_units).magnitude

or as a method on a quantity

def get_conversion_factor(self, other_units):
    # type checking etc goes here
    pint_quantity = 1 * ureg(self)  # or something like this
    return pint_quantity.to(other_units).magnitude

We could then cache those values or whatever you wanted.

[znicholls]

Where do you need conversion in the high-level API?

If people want to make plots in different units, that should be easy (I can't think of any other use cases...).

What unit conversion is happening in the low-level API?

All of the conversion when we get requests from models e.g. MAGICC's adaptor says, give me CO2 emissions in GtC/yr but the user set them as MtC / month, the low-level API has to do that conversion.

[znicholls]

What units do we need actually?

Ideally all the emissions units in that file, plus conversions for all the key IPCC metrics (AR5GWP20, AR5GWP100, AR5GTP20, AR5GTP100, AR4GWP100, AR4GWP20 etc.), concentrations units (ppm, ppb, ppt), physical units (K, C, W/m^2, kg, m, s) etc. plus all the conversions between them as, including prefixes and different times for the fluxes (e.g. per year, per month, per day, per second).

[swillner]

Hmm, ok, we shouldn't use two different unit libraries... Then I will use such a function getting offset and scaling from points, though far from optimal. But including something like that in Pint would probably better rely on Pint internals...

[znicholls]

though far from optimal

I don't understand why? If we implement something in Pint which is exactly the same as the unyt functionality, isn't that fine?

[swillner]

unyt directly exposes the offset and scaling that are used internally (in Pint these are hidden much deeper). Using two points (e.g. 1 unit and 10 unit) to calculate offset and scaling is just a work-around. Maybe it's just my desire for asthetic code then ;)
At best, I would want a unit library to expose something like a "Converter" class, that takes two units and exposes functions to convert numbers (just floats, not some wrapping Unit classes) back and forth between the two units...

[swillner]

Ok, I will have a look if we can make a PR in Pint with something like that, what do you think?

[znicholls]

Ok, I will have a look if we can make a PR in Pint with something like that, what do you think?

I like it. Good procrastination ;)

[znicholls]

@rgieseke to you for final say and then merge?

Is this all we needed in this PR?

[rgieseke]

@swillner @znicholls In general, is this ready to be merged or are you still tweaking?

[rgieseke]

Current failure is from the notebook.

[znicholls]

In general, is this ready to be merged or are you still tweaking?

Nope. Let's let Sven do his additions then I'll do the usual:

• add tests
• add docs
• add changelog entry
• add example

[swillner]

I'm not yet done with the units... Since it's just adding two lines (g and t) for each gas, should'nt we resort to defining them programmaitcally, e.g.

gases = {
    "C": "carbon,
    ...
}
for k, v in gases.items():
    ureg.define("{} = [{}]".format(k, v))
    ureg.define("g{} = g * {}".format(k, k))
    ureg.define("t{} = t * {}".format(k, k))

and then add the conversions between them?

[znicholls]

np.testing.assert_allclose

…

On Sun, 4 Nov 2018 at 11:37 am, Sven Willner ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/unit/test_units.py <#35 (comment)> : > @@ -0,0 +1,120 @@ +import numpy as np +import pytest + +from openscm.units import unit_registry +from openscm.units import DimensionalityError, UndefinedUnitError, UnitConverter + + +def test_unit_registry(): + CO2 = unit_registry("CO2") + assert CO2.to("C").magnitude == 12.0 / 44.0 yeah, I used explicit round somewhere where I thougt it's necessery... There is also an assertAlmostEqual in unittest standard package. Should we agree on a standard here? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWh-m3raQZ8XLCgy3GrTkaDV4RndKc_sks5urtF8gaJpZM4YAUT6> .

[swillner]

@rgieseke Have you disabled the "normal" merge option?

[znicholls]

Have you disabled the "normal" merge option?

yep merge commits are annoying, I prefer rebase...

[swillner]

ah, ok. will you have the honor then? ;)

[znicholls]

Nah in general I think authors shouldn't merge, for you or robert

…

On Sun, 4 Nov 2018 at 12:31 pm, Sven Willner ***@***.***> wrote: ah, ok. will you have the honor then? ;) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#35 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWh-m0V5Cum5QIfR4yXSwKCNYIGe_C03ks5urt4PgaJpZM4YAUT6> .