fixes from code review

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Unreleased
 
+- Bayesian results are now cleared whenever they become stale: on project create/load/reset, when a classical fit starts, and when a new sampling run starts. Previously the posterior overlays, plots and the "Bayesian Sampling Results" dialog could show results from a superseded run or a different project.
+- Cancelling a fit now keeps the UI locked until the worker thread actually exits, and late signals from a superseded worker are ignored. This prevents two fits from mutating the shared parameters concurrently when the minimizer cannot abort mid-run (lmfit, DFO).
+- Bayesian sampling no longer fails for data files without uncertainty/resolution columns: missing `ye` falls back to zero variances (reported by the sampler with a clear message) and the unused `xe` is no longer attached to the Q coordinate.
+- Fixed wrong parameter group/display names for the first and last layers of each assembly: the parameter-tree walker now prefers the canonical `layers` container over the `front_layer`/`back_layer` alias properties.
+- Removed the unused `corner` dependency.
 - Migrated to the new `easyscience` core API surface exposed by `reflectometry-lib`:
   - Layer removal now calls `remove_at(index)`; the lib's index-based `remove` was replaced by standard `MutableSequence.remove(value)` semantics.
   - Parameter discovery uses `get_all_parameters()`.

EasyReflectometryApp/Backends/Py/analysis.py

@@ -430,6 +430,9 @@ def _start_threaded_fit(self) -> None:
             return
 
         # Classical fitting path
+        # Discard any previous Bayesian posterior so the results dialog and the
+        # posterior-predictive chart overlays reflect this fit, not a stale run.
+        self._clear_bayesian_results()
         # Reset flags and prepare for fit using proper encapsulation
         self._fitting_logic.reset_stop_flag()
         self._fitting_logic.prepare_for_threaded_fit()
@@ -462,9 +465,25 @@ def _start_threaded_fit(self) -> None:
         self._fitter_thread.failed.connect(self._fitter_thread.deleteLater)
         self._fitter_thread.start()
 
+    def _is_stale_worker_signal(self) -> bool:
+        """Return True when a worker signal comes from a superseded worker.
+
+        A cancelled fit's thread may outlive the cancellation (lmfit/DFO cannot
+        abort mid-run); its late signals must not clobber the state of a newer
+        run. Signals from the current worker — and direct calls, where
+        ``sender()`` is None — are processed normally.
+        """
+        sender = self.sender()
+        if sender is not None and sender is not self._fitter_thread:
+            logger.info('Ignoring signal from a superseded fit worker')
+            return True
+        return False
+
     @Slot(dict)
     def _on_fit_progress(self, payload: dict) -> None:
         """Handle in-flight progress payloads emitted from the worker thread."""
+        if self._is_stale_worker_signal():
+            return
         if payload.get('sampling'):
             self._fitting_logic.on_sample_progress(payload)
         else:
@@ -474,6 +493,8 @@ def _on_fit_progress(self, payload: dict) -> None:
     @Slot(list)
     def _on_fit_finished(self, results: list) -> None:
         """Handle successful completion of threaded fit."""
+        if self._is_stale_worker_signal():
+            return
         self._fitting_logic.on_fit_finished(results)
         self._project_lib._last_fit_results = self._fitting_logic.last_fit_results
         # The threaded fit runs on a throwaway fitter's easy_science_multi_fitter,
@@ -494,6 +515,8 @@ def _on_fit_finished(self, results: list) -> None:
     @Slot(str)
     def _on_fit_failed(self, error_message: str) -> None:
         """Handle failed threaded fit."""
+        if self._is_stale_worker_signal():
+            return
         is_user_cancel = self._fitting_logic.fit_cancelled and 'cancel' in error_message.lower()
         if is_user_cancel:
             error_message = 'Fitting cancelled by user'
@@ -518,8 +541,36 @@ def _onStopFit(self) -> None:
     # Bayesian sampling dispatch and result handling
     # ------------------------------------------------------------------
 
+    def _clear_bayesian_results(self) -> None:
+        """Discard the stored posterior, rendered assets and chart overlays.
+
+        Without this, a previous run's posterior keeps the "Bayesian Sampling
+        Results" dialog branch, the credible-interval chart overlays and the
+        posterior tab alive after the model, minimizer or project has changed.
+        """
+        had_result = self._bayesian_logic.has_result
+        self._bayesian_logic.clear()
+        if self._plotting is not None:
+            self._plotting.clear_posterior_predictive()
+            self._plotting.clear_posterior_predictive_sld()
+        if had_result:
+            self.fittingChanged.emit()
+            self.heatmapChanged.emit()
+
+    @Slot()
+    def clearBayesianResults(self) -> None:
+        """Public entry point for discarding Bayesian results.
+
+        Connected by PyBackend to project create/load/reset signals so results
+        from one project can never be displayed against another.
+        """
+        self._clear_bayesian_results()
+
     def _start_threaded_sample(self) -> None:
         """Start Bayesian MCMC sampling in a background thread."""
+        # A fresh run invalidates the previous posterior immediately; if the
+        # run fails, the UI must not keep presenting the old result as current.
+        self._clear_bayesian_results()
         self._fitting_logic.prepare_for_threaded_sample()
         self.fittingChanged.emit()
 
@@ -563,6 +614,8 @@ def _start_threaded_sample(self) -> None:
     @Slot(list)
     def _on_sample_finished(self, results: list) -> None:
         """Handle successful completion of Bayesian sampling."""
+        if self._is_stale_worker_signal():
+            return
         if not results:
             logger.error('Bayesian sampling finished with empty results list')
             self._fitting_logic.on_sample_finished()

EasyReflectometryApp/Backends/Py/logic/fitting.py

@@ -176,16 +176,16 @@ def on_fit_failed(self, error_message: str) -> None:
         self.clear_fit_progress()
 
     def stop_fit(self) -> None:
-        """Request fitting to stop and clean up state."""
+        """Request the running fit/sampling to stop.
+
+        Only the stop/cancel flags are set here. The lifecycle state (running,
+        finished, results, dialog) is finalised by ``on_fit_failed`` when the
+        worker thread actually exits. Keeping ``running`` True until then keeps
+        the UI locked, so a second fit cannot be started while a non-abortable
+        minimizer (lmfit, DFO) is still mutating the shared parameters.
+        """
         self._stop_requested = True
-        self._result = None
-        self._results = []
-        self._running = False
-        self._finished = True
         self._fit_cancelled = True
-        self._fit_error_message = 'Fitting cancelled by user'
-        self._show_results_dialog = True
-        self.clear_fit_progress()
 
     def reset_stop_flag(self) -> None:
         """Reset the stop request flag before starting a new fit."""
@@ -318,21 +318,29 @@ def collect_all_experiments_datagroup(self) -> 'sc.DataGroup':
         :return: DataGroup with reflectivity coords and data.
         :rtype: sc.DataGroup
         """
+        import numpy as np
         import scipp as sc
 
         experiments = self._ordered_experiments()
         coords = {}
         data = {}
         for i, experiment in enumerate(experiments):
-            import numpy as np
-
             x_vals = np.asarray(experiment.x, dtype=float)
-            xe_vals = np.asarray(experiment.xe, dtype=float)
             y_vals = np.asarray(experiment.y, dtype=float)
-            ye_vals = np.asarray(experiment.ye, dtype=float)  # variances (σ²)
 
+            # ye holds variances (σ²), same convention as prepare_threaded_fit.
+            # Data files without an uncertainty column yield an empty/absent ye;
+            # scipp requires variances to match the values' shape, so fall back
+            # to zeros and let mcmc_sample's zero-variance handling report it.
+            ye_raw = getattr(experiment, 'ye', None)
+            ye_vals = np.asarray(ye_raw, dtype=float) if ye_raw is not None else np.zeros_like(y_vals)
+            if ye_vals.shape != y_vals.shape:
+                ye_vals = np.zeros_like(y_vals)
+
+            # No variances on the Qz coordinate: mcmc_sample only reads its
+            # values, and xe may be empty for 2/3-column data files.
             coords[f'Qz_{i}'] = sc.array(
-                dims=[f'Qz_{i}'], values=x_vals, variances=xe_vals, unit=sc.Unit('1/angstrom'),
+                dims=[f'Qz_{i}'], values=x_vals, unit=sc.Unit('1/angstrom'),
             )
             data[f'R_{i}'] = sc.array(
                 dims=[f'Qz_{i}'], values=y_vals, variances=ye_vals,
@@ -376,6 +384,7 @@ def prepare_threaded_sample(self, minimizers_logic: 'Minimizers') -> tuple:
 
     def prepare_for_threaded_sample(self) -> None:
         """Set running flags and sampling progress message before launching the worker."""
+        self.reset_stop_flag()
         self._running = True
         self._finished = False
         self._show_results_dialog = False

EasyReflectometryApp/Backends/Py/logic/parameters.py

@@ -392,17 +392,26 @@ def _children(obj) -> list:
         # Collections expose their members by iteration, not as attributes.
         if isinstance(obj, MutableSequence):
             return list(obj)
-        children = []
+        collections = []
+        others = []
         for attr_name in dir(obj):
             if attr_name.startswith('_'):
                 continue
             try:
                 value = getattr(obj, attr_name)
             except Exception:
                 continue
-            if isinstance(value, (Parameter, ModelBase, MutableSequence)):
-                children.append(value)
-        return children
+            if isinstance(value, MutableSequence):
+                collections.append(value)
+            elif isinstance(value, (Parameter, ModelBase)):
+                others.append(value)
+        # Canonical containers first. Assemblies expose alias properties
+        # (``front_layer``/``back_layer`` point into ``layers``) that sort
+        # before 'layers' alphabetically; if an alias won the first-found
+        # path, the chain would skip the LayerCollection level and the
+        # positional lookups (``path[-4]`` = assembly) would resolve to the
+        # wrong ancestor, mislabelling superphase/subphase parameters.
+        return collections + others
 
     def _visit(obj, chain: list) -> None:
         for child in _children(obj):

EasyReflectometryApp/Backends/Py/py_backend.py

@@ -154,6 +154,11 @@ def _connect_project_page(self) -> None:
         self._project.externalCreatedChanged.connect(self._relay_project_page_created)
         self._project.externalProjectLoaded.connect(self._relay_project_page_project_changed)
         self._project.externalProjectReset.connect(self._relay_project_page_project_changed)
+        # Bayesian posteriors belong to one project state: discard them on
+        # create/load/reset so stale results are never shown against new data.
+        self._project.externalCreatedChanged.connect(self._analysis.clearBayesianResults)
+        self._project.externalProjectLoaded.connect(self._analysis.clearBayesianResults)
+        self._project.externalProjectReset.connect(self._analysis.clearBayesianResults)
 
     def _connect_sample_page(self) -> None:
         self._sample.externalSampleChanged.connect(self._relay_sample_page_sample_changed)

pyproject.toml

@@ -38,7 +38,6 @@ dependencies = [
   'toml',
   'numpy',
   'matplotlib',
-  'corner>=2.2',
   'arviz>=0.18',
   'plotly>=5.0',
 ]

tests/test_analysis.py

@@ -129,7 +129,10 @@ def test_start_threaded_fit_propagates_progress_to_properties(monkeypatch, qcore
     assert fitting_changed['count'] >= 2
 
 
-def test_on_stop_fit_requests_worker_stop_without_immediate_cleanup(monkeypatch, qcore_application):
+def test_on_stop_fit_requests_worker_stop_and_keeps_ui_locked_until_thread_exits(monkeypatch, qcore_application):
+    """Cancel only requests a cooperative stop; ``running`` stays True until the
+    worker thread actually exits, so a second fit cannot start while a
+    non-abortable minimizer is still mutating the shared parameters."""
     StubWorker.instances = []
     analysis = _make_analysis(monkeypatch)
     analysis._fitting_logic.prepare_threaded_fit = MagicMock(
@@ -143,10 +146,42 @@ def test_on_stop_fit_requests_worker_stop_without_immediate_cleanup(monkeypatch,
 
     assert worker.stop_calls == 1
     assert analysis._fitter_thread is worker
+    # UI stays locked: the thread has not exited yet.
+    assert analysis.fittingRunning is True
+    assert analysis._fitting_logic.fit_cancelled is True
+
+    # Worker thread exits and reports the cancellation.
+    worker.failed.emit('Fitting cancelled by user')
+
+    assert analysis._fitter_thread is None
     assert analysis.fittingRunning is False
     assert analysis.fitErrorMessage == 'Fitting cancelled by user'
 
 
+def test_stale_worker_signals_are_ignored_after_new_fit_starts(monkeypatch, qcore_application):
+    """Late signals from a superseded worker must not clobber the current run."""
+    StubWorker.instances = []
+    analysis = _make_analysis(monkeypatch)
+    analysis._fitting_logic.prepare_threaded_fit = MagicMock(
+        return_value=('fake-fitter', ['x'], ['y'], ['w'], None)
+    )
+
+    analysis._start_threaded_fit()
+    stale_worker = StubWorker.instances[-1]
+
+    # Simulate a newer worker having taken over.
+    analysis._start_threaded_fit()
+    current_worker = StubWorker.instances[-1]
+    assert analysis._fitter_thread is current_worker
+
+    # The old worker finally exits — its failure signal must be ignored.
+    stale_worker.failed.emit('Fitting cancelled by user')
+
+    assert analysis._fitter_thread is current_worker
+    assert analysis.fittingRunning is True
+    assert analysis.fitErrorMessage in ('', None)
+
+
 def test_fitting_start_stop_emits_stop_signal_when_fit_is_running(monkeypatch, qcore_application):
     analysis = _make_analysis(monkeypatch)
     analysis._fitting_logic.prepare_for_threaded_fit()

tests/test_analysis_bayesian.py

@@ -126,6 +126,18 @@ def set_posterior_predictive_sld(self, z, median, lo, hi):
         self.sld_lo = lo
         self.sld_hi = hi
 
+    def clear_posterior_predictive(self):
+        self.posterior_q = None
+        self.posterior_median = None
+        self.posterior_lo = None
+        self.posterior_hi = None
+
+    def clear_posterior_predictive_sld(self):
+        self.sld_z = None
+        self.sld_median = None
+        self.sld_lo = None
+        self.sld_hi = None
+
 
 class StubWorker(QObject):
     finished = Signal(list)
@@ -813,6 +825,67 @@ def test_noop_when_posterior_cleared(self, analysis_with_posterior):
         assert analysis_with_posterior._plotting.posterior_q is None
 
 
+# ===================================================================
+# Bayesian state clearing
+# ===================================================================
+
+class TestBayesianStateClearing:
+    def _set_full_result(self, analysis):
+        analysis._bayesian_logic._posterior = dict(SAMPLE_POSTERIOR_2D)
+        analysis._bayesian_logic.corner_plot_url = 'file:///corner.html'
+        analysis._bayesian_logic.diagnostics = {'nDraws': 4}
+        analysis._plotting.set_posterior_predictive([1.0], [2.0], [1.5], [2.5])
+        analysis._plotting.set_posterior_predictive_sld([0.0], [1.0], [0.5], [1.5])
+
+    def test_clear_bayesian_results_discards_posterior_and_overlays(self, analysis):
+        self._set_full_result(analysis)
+        emissions = {'fitting': 0, 'heatmap': 0}
+        analysis.fittingChanged.connect(lambda: emissions.__setitem__('fitting', emissions['fitting'] + 1))
+        analysis.heatmapChanged.connect(lambda: emissions.__setitem__('heatmap', emissions['heatmap'] + 1))
+
+        analysis.clearBayesianResults()
+
+        assert analysis._bayesian_logic.has_result is False
+        assert analysis._bayesian_logic.corner_plot_url == ''
+        assert analysis._bayesian_logic.diagnostics == {}
+        assert analysis._plotting.posterior_q is None
+        assert analysis._plotting.sld_z is None
+        assert emissions['fitting'] >= 1
+        assert emissions['heatmap'] >= 1
+
+    def test_clear_without_result_does_not_emit(self, analysis):
+        emissions = []
+        analysis.fittingChanged.connect(lambda: emissions.append('fitting'))
+
+        analysis.clearBayesianResults()
+
+        assert emissions == []
+
+    def test_classical_fit_start_clears_previous_posterior(self, analysis):
+        self._set_full_result(analysis)
+        # Classical path: give the stub what _start_threaded_fit needs and make
+        # preparation fail so no worker is actually started.
+        analysis._fitting_logic.prepare_threaded_fit = lambda ml: (None, None, None, None, None)
+
+        analysis._start_threaded_fit()
+
+        assert analysis._bayesian_logic.has_result is False
+        assert analysis._plotting.posterior_q is None
+
+    def test_sampling_start_clears_previous_posterior(self, analysis):
+        StubWorker.instances = []
+        self._set_full_result(analysis)
+        analysis._minimizers_logic.set_bayesian(True)
+
+        analysis._start_threaded_fit()
+
+        # The old posterior must be gone even though the new run has not
+        # finished (a failed run must not resurrect stale results).
+        assert analysis._bayesian_logic.has_result is False
+        assert analysis._bayesian_logic.corner_plot_url == ''
+        assert analysis._plotting.posterior_q is None
+
+
 # ===================================================================
 # Bayesian sampling dispatch
 # ===================================================================