Merge pull request #647 from FAIRmat-NFDI/pint-units

Author: lukaspie

.github/workflows/nomad-requirements.yml .github/workflows/nomad-compatibility.yml.github/workflows/nomad-requirements.yml renamed to .github/workflows/nomad-compatibility.yml

@@ -1,4 +1,4 @@
-name: NOMAD dependencies compatibility
+name: NOMAD compatibility
 
 on:
   push:
@@ -16,7 +16,7 @@ env:
   python-version: 3.11
 
 jobs:
-  validate_dependencies:
+  validate_compatibility:
     runs-on: ubuntu-latest
 
     steps:
@@ -28,6 +28,18 @@ jobs:
         git clone --depth 1 --branch develop --recurse-submodules https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR.git nomad
         git submodule update --init --recursive --depth 1
 
+    - name: Compare unit definition files
+      run: |
+        diff --unified=3 nomad/nomad/units/default_en.txt src/pynxtools/units/default_en.txt || {
+          echo "::error file=src/pynxtools/units/default_en.txt::default_en.txt differs from NOMAD";
+          exit 1;
+        }
+
+        diff --unified=3 nomad/nomad/units/constants_en.txt src/pynxtools/units/constants_en.txt || {
+          echo "::error file=src/pynxtools/units/constants.txt::constants.txt differs from NOMAD";
+          exit 1;
+        }
+
     - name: Add pynxtools dependency in NOMAD test_plugins.txt
       working-directory: ./nomad
       run: |

.gitignore

@@ -203,6 +203,8 @@ cython_debug/
 !mkdocs-requirements.txt
 !src/pynxtools/nexus-version.txt
 !src/pynxtools/remote_definitions_url.txt
+!src/pynxtools/units/constants_en.txt
+!src/pynxtools/units/default_en.txt
 build/
 nexusparser.egg-info/PKG-INFO
 .python-version

CITATION.cff

@@ -4,7 +4,7 @@ message:
   If you use this software, please cite it using the
   metadata from this file.
 type: software
-version: 0.10.8
+version: 0.10.9
 authors:
   - given-names: Sherjeel
     family-names: Shabih

src/pynxtools/dataconverter/helpers.py

@@ -49,6 +49,7 @@
 class ValidationProblem(Enum):
     DifferentVariadicNodesWithTheSameName = auto()
     UnitWithoutDocumentation = auto()
+    InvalidUnit = auto()
     InvalidEnum = auto()
     OpenEnumWithNewItem = auto()
     MissingRequiredGroup = auto()
@@ -99,6 +100,13 @@ def _log(self, path: str, log_type: ValidationProblem, value: Optional[Any], *ar
             logger.info(
                 f"The unit, {path} = {value}, is being written but has no documentation."
             )
+        if log_type == ValidationProblem.InvalidUnit:
+            value = cast(Any, value)
+            log_text = f"The unit '{args[0]}' at {path} does not match with the unit category {value.unit} of '{value.name}'."
+            if len(args) == 2 and args[1] is not None:
+                log_text += f" Based on the 'transformation_type' of the field {path.replace('/@units', '')}, it should match with '{args[1]}'."
+            logger.warning(log_text)
+
         elif log_type == ValidationProblem.InvalidEnum:
             logger.warning(
                 f"The value at {path} should be one of the following: {value}."

src/pynxtools/dataconverter/validation.py

@@ -42,6 +42,7 @@
     generate_tree_from,
 )
 from pynxtools.definitions.dev_tools.utils.nxdl_utils import get_nx_namefit
+from pynxtools.units import NXUnitSet, ureg
 
 
 def validate_hdf_group_against(appdef: str, data: h5py.Group):
@@ -216,6 +217,49 @@ def best_namefit_of(
     return best_match
 
 
+def is_valid_unit_for_node(
+    node: NexusNode, unit: str, unit_path: str, hints: dict[str, Any]
+) -> None:
+    """
+    Validate whether a unit string is compatible with the expected unit category for a given NeXus node.
+
+    This function checks if the provided `unit` string matches the expected unit dimensionality
+    defined in the node's `unit` field. Special logic is applied for "NX_TRANSFORMATION", where
+    the dimensionality depends on the `transformation_type` hint.
+
+    If the unit does not match the expected dimensionality, a validation problem is logged.
+
+    Args:
+        node (NexusNode): The node containing unit metadata to validate against.
+        unit (str): The unit string to validate (e.g., "m", "eV", "1", "").
+        unit_path (str): The path to the unit in the NeXus template, used for logging.
+        hints (dict[str, Any]): Additional metadata used during validation. For example,
+            hints["transformation_type"] may be used to determine the expected unit category
+            if the node represents a transformation.
+    """
+    # Need to use a list as `NXtransformation` is a special use case
+    if node.unit == "NX_TRANSFORMATION":
+        if (transformation_type := hints.get("transformation_type")) is not None:
+            category_map: dict[str, str] = {
+                "translation": "NX_LENGTH",
+                "rotation": "NX_ANGLE",
+            }
+            node_unit_category = category_map.get(transformation_type, "NX_UNITLESS")
+        else:
+            node_unit_category = "NX_UNITLESS"
+        log_input = node_unit_category
+    else:
+        node_unit_category = node.unit
+        log_input = None
+
+    if NXUnitSet.matches(node_unit_category, unit):
+        return
+
+    collector.collect_and_log(
+        unit_path, ValidationProblem.InvalidUnit, node, unit, log_input
+    )
+
+
 def validate_dict_against(
     appdef: str, mapping: MutableMapping[str, Any], ignore_undocumented: bool = False
 ) -> bool:
@@ -607,14 +651,30 @@ def handle_field(node: NexusNode, keys: Mapping[str, Any], prev_path: str):
 
             # Check unit category
             if node.unit is not None:
-                remove_from_not_visited(f"{prev_path}/{variant}/@units")
-                if f"{variant}@units" not in keys:
-                    collector.collect_and_log(
-                        variant_path,
-                        ValidationProblem.MissingUnit,
-                        node.unit,
-                    )
-                # TODO: Check unit with pint
+                unit_path = f"{variant_path}/@units"
+                if node.unit != "NX_UNITLESS":
+                    remove_from_not_visited(unit_path)
+                    if f"{variant}@units" not in keys and (
+                        node.unit != "NX_TRANSFORMATION"
+                        or mapping.get(f"{variant_path}/@transformation_type")
+                        in ("translation", "rotation")
+                    ):
+                        collector.collect_and_log(
+                            variant_path,
+                            ValidationProblem.MissingUnit,
+                            node.unit,
+                        )
+                        break
+
+                unit = keys.get(f"{variant}@units")
+                # Special case: NX_TRANSFORMATION unit depends on `@transformation_type` attribute
+                if (
+                    transformation_type := keys.get(f"{variant}@transformation_type")
+                ) is not None:
+                    hints = {"transformation_type": transformation_type}
+                else:
+                    hints = {}
+                is_valid_unit_for_node(node, unit, unit_path, hints)
 
             field_attributes = get_field_attributes(variant, keys)
             field_attributes = _follow_link(field_attributes, variant_path)
@@ -820,9 +880,13 @@ def is_documented(key: str, tree: NexusNode) -> bool:
             and node.unit is not None
             and f"{key}/@units" not in mapping
         ):
-            collector.collect_and_log(
-                f"{key}", ValidationProblem.MissingUnit, node.unit
-            )
+            # Workaround for NX_UNITLESS of NX_TRANSFORMATION unit category
+            if node.unit != "NX_TRANSFORMATION" or mapping.get(
+                f"{key}/@transformation_type"
+            ) in ("translation", "rotation"):
+                collector.collect_and_log(
+                    f"{key}", ValidationProblem.MissingUnit, node.unit
+                )
 
         return True
 
@@ -1298,6 +1362,10 @@ def check_reserved_prefix(
     check_attributes_of_nonexisting_field(tree)
 
     for not_visited_key in not_visited:
+        if mapping.get(not_visited_key) is None:
+            # This value is not really set. Skip checking its validity.
+            continue
+
         # TODO: remove again if "@target"/"@reference" is sorted out by NIAC
         always_allowed_attributes = ("@target", "@reference")
         if not_visited_key.endswith(always_allowed_attributes):
@@ -1344,6 +1412,20 @@ def check_reserved_prefix(
                             mapping[not_visited_key],
                         )
 
+                if node.unit is not None:
+                    # Special case: NX_TRANSFORMATION unit depends on `@transformation_type` attribute
+                    if (
+                        transformation_type := mapping.get(
+                            not_visited_key.replace("/@units", "/@transformation_type")
+                        )
+                    ) is not None:
+                        hints = {"transformation_type": transformation_type}
+                    else:
+                        hints = {}
+                    is_valid_unit_for_node(
+                        node, mapping[not_visited_key], not_visited_key, hints
+                    )
+
             # parent key will be checked on its own if it exists, because it is in the list
             continue
 

src/pynxtools/nomad/parser.py

@@ -33,7 +33,6 @@
     from nomad.metainfo import MEnum, MSection
     from nomad.metainfo.util import MQuantity, MSubSectionList, resolve_variadic_name
     from nomad.parsing import MatchingParser
-    from nomad.units import ureg
     from nomad.utils import get_logger
     from pint.errors import UndefinedUnitError
 except ImportError as exc:
@@ -49,6 +48,7 @@
     get_quantity_base_name,
 )
 from pynxtools.nomad.utils import _rename_nx_for_nomad as rename_nx_for_nomad
+from pynxtools.units import ureg
 
 
 def _to_group_name(nx_node: ET.Element):

src/pynxtools/nomad/schema.py

@@ -31,7 +31,6 @@
 import h5py
 import numpy as np
 import pandas as pd
-import pint
 from ase import Atoms
 from ase.data import atomic_numbers
 from scipy.spatial import cKDTree
@@ -74,7 +73,6 @@
     from nomad.metainfo.metainfo import resolve_variadic_name
     from nomad.normalizing.common import nomad_atoms_from_ase_atoms
     from nomad.normalizing.topology import add_system, add_system_info
-    from nomad.units import ureg
     from nomad.utils import get_logger, hash, strip
 
 except ImportError as exc:
@@ -91,6 +89,7 @@
     _rename_nx_for_nomad,
     get_quantity_base_name,
 )
+from pynxtools.units import NXUnitSet, ureg
 
 # URL_REGEXP from
 # https://stackoverflow.com/questions/3809401/what-is-a-good-regular-expression-to-match-a-url
@@ -327,68 +326,6 @@ def get_nx_type(nx_type: str) -> Optional[Datatype]:
     return None
 
 
-class NXUnitSet:
-    """
-    maps from `NX_` token to dimensionality
-    None -> disable dimensionality check
-    '1' -> dimensionless quantities
-    'transformation' -> Specially handled in metainfo
-    """
-
-    mapping: dict = {
-        "NX_ANGLE": "[angle]",
-        "NX_ANY": None,
-        "NX_AREA": "[area]",
-        "NX_CHARGE": "[charge]",
-        "NX_COUNT": "1",
-        "NX_CROSS_SECTION": "[area]",
-        "NX_CURRENT": "[current]",
-        "NX_DIMENSIONLESS": "1",
-        "NX_EMITTANCE": "[length] * [angle]",
-        "NX_ENERGY": "[energy]",
-        "NX_FLUX": "1 / [time] / [area]",
-        "NX_FREQUENCY": "[frequency]",
-        "NX_LENGTH": "[length]",
-        "NX_MASS": "[mass]",
-        "NX_MASS_DENSITY": "[mass] / [volume]",
-        "NX_MOLECULAR_WEIGHT": "[mass] / [substance]",
-        "NX_PERIOD": "[time]",
-        "NX_PER_AREA": "1 / [area]",
-        "NX_PER_LENGTH": "1 / [length]",
-        "NX_POWER": "[power]",
-        "NX_PRESSURE": "[pressure]",
-        "NX_PULSES": "1",
-        "NX_SCATTERING_LENGTH_DENSITY": "1 / [area]",
-        "NX_SOLID_ANGLE": "[angle] * [angle]",
-        "NX_TEMPERATURE": "[temperature]",
-        "NX_TIME": "[time]",
-        "NX_TIME_OF_FLIGHT": "[time]",
-        "NX_TRANSFORMATION": "transformation",
-        "NX_UNITLESS": "1",
-        "NX_VOLTAGE": "[energy] / [current] / [time]",
-        "NX_VOLUME": "[volume]",
-        "NX_WAVELENGTH": "[length]",
-        "NX_WAVENUMBER": "1 / [length]",
-    }
-
-    @staticmethod
-    def normalise(value: str) -> str:
-        """
-        Normalise the given token
-        """
-        value = value.upper()
-        if not value.startswith("NX_"):
-            value = "NX_" + value
-        return value
-
-    @staticmethod
-    def is_nx_token(value: str) -> bool:
-        """
-        Check if a given token is one of NX tokens
-        """
-        return NXUnitSet.normalise(value) in NXUnitSet.mapping.keys()
-
-
 # def _to_camel_case(snake_str: str, upper: bool = False) -> str:
 #     """
 #     Take as input a snake case variable and return a camel case one
@@ -747,21 +684,16 @@ def _create_field(xml_node: ET.Element, container: Section) -> Quantity:
     # dimensionality
     nx_dimensionality = xml_attrs.get("units", None)
     if nx_dimensionality:
-        dimensionality = NXUnitSet.mapping.get(nx_dimensionality)
-        if not dimensionality and nx_dimensionality != "NX_ANY":
-            try:
-                quantity = 1 * ureg(nx_dimensionality)
-                if quantity.dimensionality == "dimensionless":
-                    dimensionality = "1"
-                else:
-                    dimensionality = str(quantity.dimensionality)
-            except (
-                pint.errors.UndefinedUnitError,
-                pint.errors.DefinitionSyntaxError,
-            ) as err:
-                raise NotImplementedError(
-                    f"Unit {nx_dimensionality} is not supported for {name}."
-                ) from err
+        if nx_dimensionality == "NX_TRANSFORMATION":
+            # TODO: Remove workaround for NX_TRANSFORMATTION
+            nx_dimensionality = "NX_ANY"
+        dimensionality = NXUnitSet.get_dimensionality(nx_dimensionality)
+        if dimensionality is not None:
+            dimensionality = str(dimensionality)
+        elif nx_dimensionality != "NX_ANY":
+            raise NotImplementedError(
+                f"Unit {nx_dimensionality} is not supported for {name}."
+            )
     else:
         dimensionality = None