From b2bcfa38584e86ddbbacadd41b5ee014362d702e Mon Sep 17 00:00:00 2001
From: Lauri Himanen <lauri.himanen@gmail.com>
Date: Mon, 29 Aug 2022 12:32:11 +0000
Subject: [PATCH] Resolve "Unit issue with delta degrees"
---
gui/src/units.js | 16 ++++++++++---
gui/src/units.spec.js | 10 +++++++--
nomad/cli/dev.py | 40 ++++++++++++++++++++-------------
nomad/metainfo/metainfo.py | 14 ++++++++++--
tests/metainfo/test_metainfo.py | 10 +++++++++
5 files changed, 68 insertions(+), 22 deletions(-)
diff --git a/gui/src/units.js b/gui/src/units.js
index f07f1bbcd1..68e8e2ef5a 100644
--- a/gui/src/units.js
+++ b/gui/src/units.js
@@ -236,15 +236,25 @@ export class Unit {
/**
* Normalizes the given expression into a format that can be parsed by MathJS.
+ *
+ * This function will replace the Pint power symbol of '**' with the symbol
+ * '^' used by MathJS. In addition, we convert any 'delta'-units (see:
+ * https://pint.readthedocs.io/en/stable/nonmult.html) into their regular
+ * counterparts: MathJS will automatically ignore the offset when using
+ * non-multiplicative units in expressions.
+ *
* @param {str} expression Expression
* @returns string Expression in normalized form
*/
normalizeExpression(expression) {
- return expression.replace('**', '^')
+ let normalized = expression.replace(/\*\*/g, '^')
+ normalized = normalized.replace(/delta_/g, '')
+ normalized = normalized.replace(/Δ/g, '')
+ return normalized
}
/**
- * Checks if the given unit has the same based dimensions as this one.
+ * Checks if the given unit has the same base dimensions as this one.
* @param {str | Unit} unit Unit to compare to
* @returns boolean Whether the units have the same base dimensions.
*/
@@ -385,7 +395,7 @@ export class Unit {
* Gets the dimension of this unit as a string. The order of the dimensions is
* fixed (determined at unit registration time).
*
- * @param {boolean} base Whether to return dimension in base units. Othwerwise
+ * @param {boolean} base Whether to return dimension in base units. Otherwise
* the original unit dimensions are used.
* @returns The dimensionality as a string, e.g. 'time^2 energy mass^-2'
*/
diff --git a/gui/src/units.spec.js b/gui/src/units.spec.js
index ea982fb8c5..ca9044c026 100644
--- a/gui/src/units.spec.js
+++ b/gui/src/units.spec.js
@@ -89,7 +89,12 @@ test.each([
['division', 'm/s', 'angstrom/femtosecond', 'Å / fs'],
['multiplication', 'm*s', 'angstrom*femtosecond', 'Å fs'],
['power with hat', 'm^2', 'angstrom^2', 'Å^2'],
- ['power with double asterisk', 'm**2', 'angstrom**2', 'Å^2'],
+ ['power with double asterisk (single)', 'm**2', 'angstrom**2', 'Å^2'],
+ ['power with double asterisk (multiple)', 'm**2 / s**2', 'angstrom**2 / ms**2', 'Å^2 / ms^2'],
+ ['explicit delta (single)', 'delta_celsius', 'delta_K', 'K'],
+ ['explicit delta (multiple)', 'delta_celsius / delta_celsius', 'delta_K / delta_K', 'K / K'],
+ ['explicit delta symbol (single)', 'Δcelsius', 'ΔK', 'K'],
+ ['explicit delta symbol (multiple)', 'Δcelsius / Δcelsius', 'ΔK / ΔK', 'K / K'],
['combined', 'm*m/s^2', 'angstrom^2/femtosecond^2', 'Å^2 / fs^2'],
['negative exponent', 's^-2', 'femtosecond^-2', 'fs^-2'],
['simple to complex with one unit', 'N', 'kg*m/s^2', '(kg m) / s^2'],
@@ -137,7 +142,8 @@ test.each([
['celsius to kelvin', 'celsius', 'kelvin', 5, 278.15],
['fahrenheit to kelvin', 'fahrenheit', 'kelvin', 5, 258.15],
['celsius to fahrenheit', 'celsius', 'fahrenheit', 5, 41],
- ['celsius to kelvin: derived unit (offset not applied)', 'joule/celsius', 'joule/kelvin', 5, 5],
+ ['celsius to kelvin: derived unit (implicit delta)', 'joule/celsius', 'joule/kelvin', 5, 5],
+ ['celsius to kelvin: derived unit (explicit delta)', 'joule/delta_celsius', 'joule/kelvin', 5, 5],
['fahrenheit to kelvin: derived unit (offset not applied)', 'joule/fahrenheit', 'joule/kelvin', 5, 9 / 5 * 5],
['celsius to fahrenheit: derived unit (offset not applied)', 'joule/celsius', 'joule/fahrenheit', 5, 5 / 9 * 5]
]
diff --git a/nomad/cli/dev.py b/nomad/cli/dev.py
index ccae421dde..13c8d296f4 100644
--- a/nomad/cli/dev.py
+++ b/nomad/cli/dev.py
@@ -380,6 +380,19 @@ def units(ctx):
aliases[unit_long_name].append(unit_name)
# For each defined dimension, get the available units if there are any.
+ def get_unit_data(unit, dimension):
+ unit_long_name = ureg.get_name(unit_name)
+ unit_abbreviation = ureg.get_symbol(unit_name)
+ unit_label = unit_long_name.replace('_', ' ')
+ unit_label = unit_label[0].upper() + unit_label[1:]
+
+ return {
+ 'name': unit_long_name,
+ 'dimension': dimension[1:-1],
+ 'label': unit_label,
+ 'abbreviation': unit_abbreviation,
+ 'aliases': aliases[unit_long_name]
+ }
dimensions = list(ureg._dimensions.keys())
unit_list = []
for dimension in dimensions:
@@ -389,19 +402,8 @@ def units(ctx):
continue
else:
for unit in units:
- key = str(unit)
- unit_long_name = ureg.get_name(key)
- unit_abbreviation = ureg.get_symbol(key)
- unit_label = unit_long_name.replace('_', ' ')
- unit_label = unit_label[0].upper() + unit_label[1:]
-
- unit_list.append({
- 'name': unit_long_name,
- 'dimension': dimension[1:-1],
- 'label': unit_label,
- 'abbreviation': unit_abbreviation,
- 'aliases': aliases[unit_long_name]
- })
+ unit_name = str(unit)
+ unit_list.append(get_unit_data(unit_name, dimension))
# Some units need to be added manually.
unit_list.extend([
@@ -503,8 +505,16 @@ def units(ctx):
is_number = False
if not is_operator and not is_number:
units.add(part)
- # for unit in units:
- # assert unit in unit_names, 'The unit "{}" is not defined in the unit definitions.'.format(unit)
+
+ # Check that the defined units do not contain 'delta_' or 'Δ' in them. This is
+ # reserved to indicate that a quantity should be treated without offset.
+ # MathJS does not have explicit support for these delta-units, but instead
+ # uses them implicitly when non-multiplicative units appear in expressions.
+ for unit in unit_names:
+ assert 'delta_' not in unit and 'Δ' not in unit, (
+ f'Invalid unit name {unit}. "delta_" and "Δ" are reserved for unit variants '
+ 'with no offset, but MathJS does not have support for these units.'
+ )
# Print unit conversion table and unit systems as a Javascript source file
output = '''/*
diff --git a/nomad/metainfo/metainfo.py b/nomad/metainfo/metainfo.py
index aa288097f8..e98573a7b4 100644
--- a/nomad/metainfo/metainfo.py
+++ b/nomad/metainfo/metainfo.py
@@ -423,10 +423,20 @@ class _Dimension(DataType):
class _Unit(DataType):
def set_normalize(self, section, quantity_def: 'Quantity', value):
+ # Explicitly providing a Pint delta-unit is not currently allowed.
+ # Implicit conversions are fine as MathJS on the frontend supports them.
+ def has_delta(unit_string):
+ return 'delta_' in unit_string or 'Δ' in unit_string
+
+ delta_error = TypeError('Explicit Pint "delta"-units are not yet supported.')
if isinstance(value, str):
+ if has_delta(value):
+ raise delta_error
value = units.parse_units(value)
-
- elif not isinstance(value, pint.unit._Unit):
+ elif isinstance(value, pint.unit._Unit):
+ if has_delta(str(value)):
+ raise delta_error
+ else:
raise TypeError('Units must be given as str or pint Unit instances.')
return value
diff --git a/tests/metainfo/test_metainfo.py b/tests/metainfo/test_metainfo.py
index 7a2b46dfa4..c300d33508 100644
--- a/tests/metainfo/test_metainfo.py
+++ b/tests/metainfo/test_metainfo.py
@@ -182,6 +182,16 @@ class TestM2:
def test_unit(self):
assert System.lattice_vectors.unit is not None
+ def test_unit_explicit_delta(self):
+ with pytest.raises(TypeError):
+ Quantity(type=np.dtype(np.float64), unit='delta_degC / hr')
+ with pytest.raises(TypeError):
+ Quantity(type=np.dtype(np.float64), unit='ΔdegC / hr')
+ Quantity(type=np.dtype(np.float64), unit='degC / hr')
+ with pytest.raises(TypeError):
+ Quantity(type=np.dtype(np.float64), unit=ureg.delta_degC / ureg.hour)
+ Quantity(type=np.dtype(np.float64), unit=ureg.degC / ureg.hour)
+
def test_extension(self):
assert getattr(Run, 'x_vasp_raw_format', None) is not None
assert 'x_vasp_raw_format' in Run.m_def.all_quantities
--
GitLab