test: add a regression test for stratigraphic values

loopstructural/gui/map2loop_tools/fault_topology_widget.py

@@ -174,7 +174,7 @@ def _run_topology(self):
                     # not just the ones map2loop found a relationship for. A fault with
                     # no detected topological relationship is still a real fault and
                     # must not be dropped from the fault topology.
-                    new_faults = set(str(v) for v in gdf['ID'].unique())
+                    new_faults = {str(v) for v in gdf['ID'].unique()}
 
                     # Add new faults; never remove existing ones here, so faults
                     # without a detected relationship (or ones the user added
@@ -209,9 +209,7 @@ def _run_topology(self):
                                 else:
                                     f1 = str(row.iloc[0])
                                     f2 = str(row.iloc[1])
-                                ft.update_fault_relationship(
-                                    f1, f2, FaultRelationshipType.ABUTTING
-                                )
+                                ft.update_fault_relationship(f1, f2, FaultRelationshipType.ABUTTING)
                             except Exception:
                                 pass
 

loopstructural/gui/modelling/model_definition/bounding_box.py

@@ -32,11 +32,11 @@ def __init__(self, parent=None, data_manager=None):
             self.selectFromCurrentLayerButton, "mActionZoomToLayer.svg", "Select from Current Layer"
         )
         self._style_tool_button(
-            self.useCurrentViewExtentButton, "mActionSetToCanvasExtent.svg", "Use Current View Extent"
-        )
-        self._style_tool_button(
-            self.drawOnMapButton, "mActionAddBasicRectangle.svg", "Draw on Map"
+            self.useCurrentViewExtentButton,
+            "mActionSetToCanvasExtent.svg",
+            "Use Current View Extent",
         )
+        self._style_tool_button(self.drawOnMapButton, "mActionAddBasicRectangle.svg", "Draw on Map")
         self.drawOnMapButton.setCheckable(True)
         self.drawOnMapButton.clicked.connect(self.drawOnMap)
         self._draw_extent_tool = None

loopstructural/gui/visualisation/feature_list_widget.py

@@ -56,9 +56,7 @@ def __init__(self, parent=None, *, model_manager=None, viewer=None, data_manager
         self.data_manager = data_manager
 
         # Add buttons
-        self.addBoundingBoxButton = self._make_tool_button(
-            "extents.svg", "Add Model Bounding Box"
-        )
+        self.addBoundingBoxButton = self._make_tool_button("extents.svg", "Add Model Bounding Box")
         self.addFaultSurfacesButton = self._make_custom_icon_tool_button(
             "fault.svg", "Add Fault Surfaces"
         )
@@ -724,7 +722,12 @@ def _extract_line_xy(self, layer) -> Optional[np.ndarray]:
             except Exception:
                 target_crs = None
         source_crs = layer.sourceCrs()
-        if target_crs is not None and target_crs.isValid() and source_crs.isValid() and source_crs != target_crs:
+        if (
+            target_crs is not None
+            and target_crs.isValid()
+            and source_crs.isValid()
+            and source_crs != target_crs
+        ):
             geom = QgsGeometry(geom)
             geom.transform(QgsCoordinateTransform(source_crs, target_crs, QgsProject.instance()))
 

loopstructural/main/m2l_api.py

@@ -98,7 +98,9 @@ def extract_basal_contacts(
     unit_name_col = 'UNITNAME' if 'UNITNAME' in geology.columns else unit_name_field
     if unit_name_col and unit_name_col in geology.columns:
         geology_unit_names = {str(v).strip() for v in geology[unit_name_col].dropna().unique()}
-        stratigraphic_names = {str(name).strip() for name in stratigraphic_order if name is not None}
+        stratigraphic_names = {
+            str(name).strip() for name in stratigraphic_order if name is not None
+        }
         ignored_names = {str(unit).strip() for unit in ignore_units if unit is not None}
         missing_from_column = sorted(geology_unit_names - stratigraphic_names - ignored_names)
         if missing_from_column:

loopstructural/main/model_manager.py

@@ -549,11 +549,9 @@ def update_foliation_features(self):
             data = []
             groupname = group.name
             stratigraphic_column[groupname] = {}
-            for u in reversed(group.units):
+            for u in group.units:
                 unit_data = self.stratigraphy.get(u.name, None)
-                if unit_data is None:
-                    continue
-                else:
+                if unit_data is not None:
                     if 'contact' in unit_data:
                         contact = unit_data['contact']
                         if not contact.empty:

tests/qgis/test_stratigraphic_value_consistency.py

@@ -0,0 +1,149 @@
+"""Regression test for the training-value / isovalue direction bug.
+
+`GeologicalModelManager.update_foliation_features` assigns a scalar `val` to
+each unit's basal contact before handing the data to the interpolator.
+`StratigraphicColumn.get_isovalues` (LoopStructural core) later decides which
+name to stamp on each extracted isosurface, using its own idea of which
+value belongs to which unit.
+
+
+Both walk `reversed(group.units)`, accumulating cumulative thickness the
+same way, so a unit's own training value must equal `u.min()` -- the
+cumulative thickness *before* that unit's own thickness is added. This is
+also each unit's true base: `add_unit(..., where='top')` (the default)
+appends to the end of the column, so building a column correctly means
+adding the truly oldest unit first and progressively younger ones after --
+each unit's own base is the boundary shared with the next-older neighbour
+processed just before it, i.e. `min()`. See LoopStructural's own
+`test_get_isovalues_multi_unit_group` (`tests/unit/modelling/
+test_stratigraphic_column.py`), whose comment states this explicitly: "the
+base of the oldest unit in a group is 0".
+
+If training and `get_isovalues()` disagree on this, every extracted surface
+gets labelled with the wrong unit while keeping correct geometry -- see the
+"stratigraphic column was reversed" fixes in model_manager.py (2025-07-21)
+and the widget (2025-08-21, reverted 2025-09-08). This has flipped back and
+forth as this plugin and LoopStructural evolved independently; this test
+pins the invariant so a future change on either side fails loudly here
+instead of silently inverting a user's model.
+
+Note this is a separate concern from whether a stratigraphic column's units
+were themselves *added* in the correct oldest-to-youngest order -- if they
+weren't, `min()`/`max()` stop corresponding to true geological base/top no
+matter what training does, and the fix is to reorder the column's units,
+not to change which value training uses.
+"""
+
+import pandas as pd
+import pytest
+from LoopStructural import StratigraphicColumn
+
+from loopstructural.main.model_manager import GeologicalModelManager
+
+
+def _contact(unit_name):
+    """A minimal single-point basal contact, tagged with its unit name so
+    the test can recover which row came from which unit after the group
+    DataFrames get concatenated."""
+    return pd.DataFrame({'X': [0.0], 'Y': [0.0], 'Z': [0.0], 'source_unit': [unit_name]})
+
+
+@pytest.fixture
+def manager(monkeypatch):
+    manager = GeologicalModelManager()
+
+    captured_calls = []
+
+    def fake_create_and_add_foliation(name, data=None, **kwargs):
+        captured_calls.append(data)
+        return object()  # stand-in foliation, only passed back into add_unconformity
+
+    monkeypatch.setattr(manager.model, 'create_and_add_foliation', fake_create_and_add_foliation)
+    monkeypatch.setattr(manager.model, 'add_unconformity', lambda *a, **k: None)
+    manager._captured_calls = captured_calls
+    return manager
+
+
+class TestTrainingValueMatchesIsovalue:
+    def test_single_group_three_units(self, manager):
+        column = StratigraphicColumn()
+        column.clear(basement=False)  # single flat group, no unconformities
+        column.add_unit(name='oldest', thickness=100.0, where='top')
+        column.add_unit(name='middle', thickness=200.0, where='top')
+        column.add_unit(name='youngest', thickness=300.0, where='top')
+
+        manager.stratigraphic_column = column
+        for name in ('oldest', 'middle', 'youngest'):
+            manager.stratigraphy[name]['contact'] = _contact(name)
+
+        manager.update_foliation_features()
+
+        training_values = self._training_values_by_unit(manager._captured_calls)
+        expected_values = {name: entry['value'] for name, entry in column.get_isovalues().items()}
+
+        for unit_name in ('oldest', 'middle', 'youngest'):
+            assert training_values[unit_name] == pytest.approx(expected_values[unit_name]), (
+                f"'{unit_name}' was trained with val={training_values[unit_name]} but "
+                f"get_isovalues() will label the value={expected_values[unit_name]} surface "
+                f"with this unit's name -- the trained field and the isosurface labels "
+                f"disagree on direction, so extracted surfaces will get the wrong unit name."
+            )
+
+    def test_two_groups_split_by_unconformity(self, manager):
+        column = StratigraphicColumn()
+        column.clear(basement=False)
+        column.add_unit(name='basin_floor', thickness=50.0, where='top')
+        column.add_unit(name='basin_fill', thickness=150.0, where='top')
+        column.add_unconformity(name='regional_unconformity', where='top')
+        column.add_unit(name='cover_lower', thickness=80.0, where='top')
+        column.add_unit(name='cover_upper', thickness=120.0, where='top')
+
+        manager.stratigraphic_column = column
+        for name in ('basin_floor', 'basin_fill', 'cover_lower', 'cover_upper'):
+            manager.stratigraphy[name]['contact'] = _contact(name)
+
+        manager.update_foliation_features()
+
+        training_values = self._training_values_by_unit(manager._captured_calls)
+        expected_values = {name: entry['value'] for name, entry in column.get_isovalues().items()}
+
+        for unit_name in ('basin_floor', 'basin_fill', 'cover_lower', 'cover_upper'):
+            assert training_values[unit_name] == pytest.approx(expected_values[unit_name])
+
+    def test_undigitised_unit_does_not_shift_later_units_in_group(self, manager):
+        """Regression test for a real bug: a unit with no digitised contact
+        or orientation data (e.g. a "Top" unit nobody has mapped points for)
+        must still contribute its own thickness to `val` for every unit
+        that follows it in the group -- `update_foliation_features` used to
+        `continue` past an undigitised unit before accumulating its
+        thickness, which shifted every later unit's trained value relative
+        to what `get_isovalues()` expects.
+        """
+        column = StratigraphicColumn()
+        column.clear(basement=False)
+        column.add_unit(name='basin_floor', thickness=50.0, where='top')
+        column.add_unit(name='basin_fill', thickness=150.0, where='top')
+        column.add_unit(name='Top', thickness=999.0, where='top')
+
+        manager.stratigraphic_column = column
+        for name in ('basin_floor', 'basin_fill'):
+            manager.stratigraphy[name]['contact'] = _contact(name)
+        # 'Top' deliberately has no entry in manager.stratigraphy at all.
+
+        manager.update_foliation_features()
+
+        training_values = self._training_values_by_unit(manager._captured_calls)
+        expected_values = {name: entry['value'] for name, entry in column.get_isovalues().items()}
+
+        for unit_name in ('basin_floor', 'basin_fill'):
+            assert training_values[unit_name] == pytest.approx(expected_values[unit_name]), (
+                f"'{unit_name}' was trained with val={training_values[unit_name]} but "
+                f"get_isovalues() expects value={expected_values[unit_name]} -- an "
+                f"undigitised unit earlier in the group must still shift later units' "
+                f"trained values by its own thickness."
+            )
+
+    @staticmethod
+    def _training_values_by_unit(captured_calls):
+        combined = pd.concat(captured_calls, ignore_index=True)
+        return dict(zip(combined['source_unit'], combined['val']))