fix: Conductivity unit handling: verify in readers, normalise in pipeline

pyproject.toml

@@ -103,3 +103,9 @@ where = ["."]
 [tool.setuptools.package-data]
 "seasenselib.knowledge" = ["**/*.json"]
 "seasenselib.config.pipeline" = ["*.json"]
+
+[tool.coverage.run]
+source = ["seasenselib"]
+
+[tool.coverage.report]
+omit = ["tests/*"]

seasenselib/config/pipeline/default.json

@@ -16,7 +16,8 @@
       "name": "unit_handling",
       "config": {
         "handlers": [
-          "normalize"
+          "normalize",
+          "conductivity_normalize"
         ]
       }
     },

seasenselib/config/pipeline/full.json

@@ -17,6 +17,7 @@
       "config": {
         "handlers": [
           "normalize",
+          "conductivity_normalize",
           "convert"
         ]
       }

seasenselib/knowledge/pipeline/metadata_enrichment/parameters_metadata.json

@@ -10,7 +10,7 @@
     "measurement_type": "Measured",
     "short_name": "COND",
     "standard_name": "sea_water_electrical_conductivity",
-    "units": "S m-1"
+    "units": "mS cm-1"
   },
   "correlation": {
     "long_name": "Correlation",
@@ -104,13 +104,13 @@
     "long_name": "Potential Temperature \u03b8",
     "measurement_type": "Derived",
     "standard_name": "sea_water_potential_temperature",
-    "units": "degC"
+    "units": "degree_C"
   },
   "conservative_temperature": {
     "long_name": "Conservative Temperature",
     "measurement_type": "Derived",
     "standard_name": "sea_water_conservative_temperature",
-    "units": "degC"
+    "units": "degree_C"
   },
   "absolute_salinity": {
     "long_name": "Absolute Salinity",
@@ -148,12 +148,13 @@
     "units": "m/s"
   },
   "temperature": {
+    "_comment": "Units are degree_C, assumed ITS-90 for all modern instruments. A variable named temperature_68 would indicate IPTS-68 scale and must be handled separately. See https://exchange-format.readthedocs.io/en/latest/parameters.html#ctdtmp",
     "coverage_content_type": "physicalMeasurement",
     "long_name": "Temperature",
     "measurement_type": "Measured",
     "short_name": "WT",
     "standard_name": "sea_water_temperature",
-    "units": "ITS-90, deg C"
+    "units": "degree_C"
   },
   "time": {
     "coverage_content_type": "coordinate",

seasenselib/knowledge/pipeline/unit_handling/expected_units.json

@@ -1,6 +1,6 @@
 {
   "battery_voltage": "volts",
-  "conductivity": "S m-1",
+  "conductivity": "mS cm-1",
   "correlation": "unitless",
   "density": "kg m-3",
   "depth": "meters",
@@ -13,11 +13,11 @@
   "magnitude": "m/s",
   "north_velocity": "m/s",
   "pitch": "degrees",
-  "potential_temperature": "K",
+  "potential_temperature": "degree_C",
   "power_supply_input": "V",
   "pressure": "dbar",
   "roll": "degrees",
   "speed_of_sound": "m/s",
-  "temperature": "K",
+  "temperature": "degree_C",
   "up_velocity": "m/s"
 }

seasenselib/knowledge/pipeline/unit_handling/unit_normalizations.json

@@ -22,7 +22,8 @@
   "degree": "degrees",
   "V": "volts",
   "volt": "volts",
-  "Sverdrup/m": "Sv/m",
+  "S/m": "S m-1",
+  "mS/cm": "mS cm-1",
   "PetaWatts": "PW",
   "petawatts": "PW",
   "kg/m³": "kg m-3",

seasenselib/parameters.py

@@ -43,9 +43,12 @@
 
 # Meta data should use standardized values from https://cfconventions.org/
 metadata = {
+    # Units are degree_C, assumed ITS-90 for all modern instruments.
+    # A variable named temperature_68 indicates IPTS-68 scale and must be handled separately.
+    # See https://exchange-format.readthedocs.io/en/latest/parameters.html#ctdtmp
     TEMPERATURE: {
         'long_name': "Temperature",
-        'units': "ITS-90, deg C",
+        'units': "degree_C",
         'coverage_content_type': 'physicalMeasurement',
         'standard_name': 'sea_water_temperature',
         'short_name': "WT",
@@ -62,7 +65,7 @@
     CONDUCTIVITY: {
         'long_name': "Conductivity",
         'coverage_content_type': 'physicalMeasurement',
-        'units': "S m-1",
+        'units': "mS cm-1",
         'standard_name': 'sea_water_electrical_conductivity',
         'short_name': "COND",
         'measurement_type': "Measured",
@@ -117,13 +120,13 @@
     },
     POTENTIAL_TEMPERATURE: {
         'long_name': 'Potential Temperature θ',
-        'units': 'degC',
+        'units': 'degree_C',
         'standard_name': 'sea_water_potential_temperature',
         'measurement_type': 'Derived',
     },
     CONSERVATIVE_TEMPERATURE: {
         'long_name': 'Conservative Temperature',
-        'units': 'degC',
+        'units': 'degree_C',
         'standard_name': 'sea_water_conservative_temperature',
         'measurement_type': 'Derived',
     },

seasenselib/pipeline/derivation/stage.py

@@ -136,4 +136,33 @@ def configure(self, config: Dict[str, Any]) -> None:
             )
 
     def process(self, context: StageContext) -> StageContext:
+        self._warn_if_conductivity_in_sm(context.dataset)
         return self._derivation.process(context)
+
+    @staticmethod
+    def _warn_if_conductivity_in_sm(ds) -> None:
+        """Warn if a conductivity variable arrives with S/m units.
+
+        The conductivity_normalize handler in unit_handling should have
+        converted it to mS cm-1 before this stage runs.  If it hasn't,
+        any future salinity-from-conductivity derivation would produce
+        values that are wrong by a factor of 10.
+        """
+        import warnings
+        import seasenselib.parameters as params
+        _SM_UNITS = frozenset({"S/m", "S m-1"})
+        cond_key = params.CONDUCTIVITY
+        for var_name in ds.data_vars:
+            if var_name == cond_key or (
+                var_name.startswith(cond_key + "_") and var_name[len(cond_key) + 1:].isdigit()
+            ):
+                units = ds[var_name].attrs.get("units", "")
+                if units in _SM_UNITS:
+                    warnings.warn(
+                        f"Variable '{var_name}' has units '{units}' at the derivation stage. "
+                        "Conductivity should be in mS cm-1 before derivation. "
+                        "Check that the 'conductivity_normalize' handler is enabled "
+                        "in the unit_handling stage.",
+                        UserWarning,
+                        stacklevel=4,
+                    )

seasenselib/pipeline/unit_handling/handlers/__init__.py

@@ -6,8 +6,10 @@
 
 from .unit_normalizer import UnitNormalizer
 from .unit_converter import UnitConverter
+from .conductivity_normalizer import ConductivityNormalizer
 
 __all__ = [
     "UnitNormalizer",
     "UnitConverter",
+    "ConductivityNormalizer",
 ]

seasenselib/pipeline/unit_handling/handlers/conductivity_normalizer.py

@@ -0,0 +1,82 @@
+"""
+Conductivity normalizer.
+
+Converts conductivity variables to the canonical unit mS cm-1 before the
+derivation stage runs.  Uses magnitude-based inference to detect and warn
+about declared-vs-actual mismatches.
+"""
+
+from __future__ import annotations
+
+import logging
+from typing import List, Tuple
+
+import xarray as xr
+
+from .utils import _base_name
+from seasenselib.readers.utils.conductivity_units import (
+    infer_conductivity_unit,
+    to_mS_cm,
+)
+
+logger = logging.getLogger(__name__)
+
+_TARGET_UNIT = "mS cm-1"
+
+
+class ConductivityNormalizer:
+    """Convert conductivity variables to mS cm-1."""
+
+    def normalize(self, ds: xr.Dataset) -> Tuple[xr.Dataset, List[str]]:
+        """Normalise all conductivity variables to mS cm-1.
+
+        Parameters
+        ----------
+        ds : xr.Dataset
+
+        Returns
+        -------
+        tuple[xr.Dataset, list[str]]
+            Updated dataset and list of normalisation records
+            (``"var: old_unit -> mS cm-1"``).
+        """
+        normalizations: List[str] = []
+
+        for var_name in list(ds.data_vars):
+            if _base_name(var_name) != "conductivity":
+                continue
+
+            current_units = ds[var_name].attrs.get("units", "")
+            if not current_units:
+                continue
+            if current_units == _TARGET_UNIT:
+                continue
+
+            values = ds[var_name].values
+            try:
+                inferred = infer_conductivity_unit(values, declared=current_units)
+                converted, canonical = to_mS_cm(values, inferred)
+            except ValueError as exc:
+                logger.warning(
+                    "ConductivityNormalizer: cannot convert '%s' (units='%s'): %s",
+                    var_name,
+                    current_units,
+                    exc,
+                )
+                continue
+
+            saved_attrs = dict(ds[var_name].attrs)
+            dims = ds[var_name].dims
+            ds[var_name] = (dims, converted)
+            ds[var_name].attrs.update(saved_attrs)
+            ds[var_name].attrs["units"] = canonical
+            ds[var_name].attrs["conductivity_normalised_from"] = current_units
+            normalizations.append(f"{var_name}: {current_units} -> {canonical}")
+            logger.info(
+                "Normalised conductivity '%s': %s -> %s",
+                var_name,
+                current_units,
+                canonical,
+            )
+
+        return ds, normalizations

seasenselib/pipeline/unit_handling/handlers/unit_normalizer.py

@@ -33,7 +33,7 @@ def __init__(self, strict: bool = False, auto_convert: bool = True,
 
     def normalize(self, ds: xr.Dataset) -> Tuple[xr.Dataset, List[str], List[str]]:
         issues: List[str] = []
-        conversions: List[str] = []
+        relabels: List[str] = []
 
         for var_name in ds.data_vars:
             var = ds[var_name]
@@ -53,7 +53,19 @@ def normalize(self, ds: xr.Dataset) -> Tuple[xr.Dataset, List[str], List[str]]:
                 old_units = current_units
                 new_units = self.unit_normalizations[current_units]
                 var.attrs['units'] = new_units
-                conversions.append(f"{var_name}: {old_units} -> {new_units}")
-                logger.debug("Normalized units for '%s': %s -> %s", var_name, old_units, new_units)
+                relabels.append(f"{var_name}: {old_units} -> {new_units}")
+                logger.debug("Relabelled units for '%s': %s -> %s", var_name, old_units, new_units)
+            elif _base_name(var_name) in self.expected_units:
+                expected = self.expected_units[_base_name(var_name)]
+                if current_units != expected:
+                    msg = (
+                        f"Variable '{var_name}' has unit '{current_units}' "
+                        f"but expected '{expected}'"
+                    )
+                    issues.append(msg)
+                    if self.strict:
+                        raise ValueError(msg)
+                    warnings.warn(msg)
+                    logger.debug(msg)
 
-        return ds, issues, conversions
+        return ds, issues, relabels

seasenselib/pipeline/unit_handling/stage.py

@@ -11,6 +11,7 @@
 from ..base import Stage, StageContext
 from .handlers.unit_normalizer import UnitNormalizer
 from .handlers.unit_converter import UnitConverter
+from .handlers.conductivity_normalizer import ConductivityNormalizer
 
 
 class UnitHandlingStage(Stage):
@@ -19,6 +20,7 @@ class UnitHandlingStage(Stage):
     def __init__(self):
         self._normalizer = UnitNormalizer()
         self._converter = UnitConverter()
+        self._cond_normalizer = ConductivityNormalizer()
 
     def name(self) -> str:
         return "unit_handling"
@@ -28,9 +30,11 @@ def configure(self, config: Dict[str, Any]) -> None:
         if isinstance(handlers, list) and handlers:
             self._enable_normalize = 'normalize' in handlers
             self._enable_convert = 'convert' in handlers
+            self._enable_cond_normalize = 'conductivity_normalize' in handlers
         else:
             self._enable_normalize = True
             self._enable_convert = False
+            self._enable_cond_normalize = True
 
         # Pass config through to normalizer / converter
         normalizer_cfg = dict(config)
@@ -53,13 +57,19 @@ def process(self, context: StageContext) -> StageContext:
         from ..utils import record_handler_applied
 
         if getattr(self, '_enable_normalize', True):
-            ds, issues, conversions = self._normalizer.normalize(ds)
+            ds, issues, relabels = self._normalizer.normalize(ds)
             if issues:
                 context.metadata['unit_validation_issues'] = issues
-            if conversions:
-                context.metadata.setdefault('unit_conversions', []).extend(conversions)
+            if relabels:
+                context.metadata.setdefault('unit_conversions', []).extend(relabels)
             record_handler_applied(context.metadata, self.name(), "normalize")
 
+        if getattr(self, '_enable_cond_normalize', True):
+            ds, cond_normalizations = self._cond_normalizer.normalize(ds)
+            if cond_normalizations:
+                context.metadata.setdefault('unit_conversions', []).extend(cond_normalizations)
+            record_handler_applied(context.metadata, self.name(), "conductivity_normalize")
+
         if getattr(self, '_enable_convert', False):
             ds, conversions = self._converter.convert(ds)
             if conversions:

seasenselib/readers/sbe_ascii_reader.py

@@ -10,6 +10,7 @@
 import xarray as xr
 
 from seasenselib.readers.base import AbstractReader
+from seasenselib.readers.utils.conductivity_units import infer_conductivity_unit
 import seasenselib.parameters as params
 
 logger = logging.getLogger(__name__)
@@ -152,9 +153,24 @@ def _create_xarray_dataset(self, df, metadata, sample_interval, instrument_type)
         """
         ds = xr.Dataset.from_dataframe(df)
 
+        # SBE37 ASCII output is S/m for conductivity. Verify magnitude matches
+        # declared unit; the pipeline unit_handling stage normalises to mS cm-1.
+        cond_values = ds['conductivity'].values
+        declared_unit = "S/m"
+        try:
+            infer_conductivity_unit(cond_values, declared=declared_unit)
+        except ValueError as exc:
+            logger.warning(
+                "Could not verify conductivity units against magnitude: %s",
+                exc,
+            )
+        ds['conductivity'].attrs = {
+            'units': declared_unit,
+            'conductivity_unit_source': "SBE37 ASC format outputs S/m",
+        }
+
         # Add minimal units from raw data (stages will enrich with CF metadata)
-        ds['temperature'].attrs = {'units': '°C'}
-        ds['conductivity'].attrs = {'units': 'S/m'}
+        ds['temperature'].attrs = {'units': 'degree_C'}
         if 'pressure' in ds.data_vars:
             ds['pressure'].attrs = {'units': 'dbar'}