Re #27966 Print averaged delta in SANS beam centre finder

Change the SANS Beam Centre Finder to prints the average residual change
between two quarters. This allows for direct comparisons, previously if
a user increased the radius limit it would appear to perform worse,
despite actually doing better. This was because the additional number of
points was not accounted for.

Framework/PythonInterface/plugins/algorithms/WorkflowAlgorithms/SANS/SANSBeamCentreFinder.py

@@ -173,14 +173,17 @@ class SANSBeamCentreFinder(DataProcessorAlgorithm):
             if verbose:
                 self._rename_and_group_workspaces(i, output_workspaces)
 
-            diff_left_right.append(self._calculate_residuals(sample_quartiles[MaskingQuadrant.LEFT],
-                                                             sample_quartiles[MaskingQuadrant.RIGHT]))
-            diff_top_bottom.append(self._calculate_residuals(sample_quartiles[MaskingQuadrant.TOP],
-                                                             sample_quartiles[MaskingQuadrant.BOTTOM]))
-
-            self.logger.notice("Itr {0}: ( {1:.3f}, {2:.3f} )  SX={3:.5f}  SY={4:.5f}".
-                               format(i, self.scale_1 * centre_lr, self.scale_2 * centre_tb,
-                                      diff_left_right[i], diff_top_bottom[i]))
+            lr_results = self._calculate_residuals(sample_quartiles[MaskingQuadrant.LEFT],
+                                                   sample_quartiles[MaskingQuadrant.RIGHT])
+            tb_results = self._calculate_residuals(sample_quartiles[MaskingQuadrant.TOP],
+                                                   sample_quartiles[MaskingQuadrant.BOTTOM])
+
+            self._print_results(lr_results=lr_results, tb_results=tb_results,
+                                centre_lr=centre_lr, centre_tb=centre_tb, iteration=i)
+
+            diff_left_right.append(lr_results.total_residual)
+            diff_top_bottom.append(tb_results.total_residual)
+
             if i == 0:
                 self._plot_current_result(output_workspaces)
             else:
@@ -205,6 +208,20 @@ class SANSBeamCentreFinder(DataProcessorAlgorithm):
                 self.logger.notice("Out of iterations, new coordinates may not be the best")
         return centre_lr_hold, centre_tb_hold
 
+    def _print_results(self, iteration, centre_lr, centre_tb, lr_results, tb_results):
+        scaled_lr = self.scale_1 * centre_lr
+        scaled_tb = self.scale_2 * centre_tb
+
+        avg_lr_residual = lr_results.total_residual / lr_results.num_points_considered
+        avg_tb_residual = tb_results.total_residual / tb_results.num_points_considered
+
+        iter_details = "Itr {:02d}: ({:7.3f}, {:7.3f})  SX={:<8.4f}\tSY={:<8.4f}\t Points: {:3d} (Unaligned: {:2d})" \
+            .format(iteration, scaled_lr, scaled_tb,
+                    avg_lr_residual, avg_tb_residual,
+                    lr_results.num_points_considered, lr_results.num_points_considered)
+
+        self.logger.notice(iter_details)
+
     def _plot_current_result(self, output_workspaces):
         if can_plot_beamcentrefinder():
             break_loop = self._plot_workspaces(output_workspaces, self.state.data.sample_scatter)
@@ -353,18 +370,24 @@ class SANSBeamCentreFinder(DataProcessorAlgorithm):
         B_vals_dict = dict(zip(qvalsBX, yvalsBX))
 
         residue = 0.0
+        mismatched_points = 0
         for key in B_vals_dict:
             if key not in A_vals_dict:
                 A_vals_dict[key] = 0.0
+                mismatched_points += 1
 
         for key in A_vals_dict:
             if key not in B_vals_dict:
                 B_vals_dict[key] = 0.0
+                mismatched_points += 1
+
+        assert len(A_vals_dict) == len(B_vals_dict)
 
         for key in A_vals_dict and B_vals_dict:
             residue += pow(A_vals_dict[key] - B_vals_dict[key], 2)
 
-        return residue
+        return _ResidualsDetails(mismatched_points=mismatched_points, num_points_considered=len(A_vals_dict),
+                                 total_residual=residue)
 
     def _get_progress(self):
         return Progress(self, start=0.0, end=1.0, nreports=10)
@@ -374,5 +397,12 @@ class WorkspaceContainsNanValues(Exception):
     pass
 
 
+class _ResidualsDetails(object):
+    def __init__(self, num_points_considered, mismatched_points, total_residual):
+        self.num_points_considered = num_points_considered
+        self.mismatched_points = mismatched_points
+        self.total_residual = total_residual
+
+
 # Register algorithm with Mantid
 AlgorithmFactory.subscribe(SANSBeamCentreFinder)

Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/sans/SANSBeamCentreFinderTest.py

+# Mantid Repository : https://github.com/mantidproject/mantid
+#
+# Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI,
+#     NScD Oak Ridge National Laboratory, European Spallation Source
+#     & Institut Laue - Langevin
+# SPDX - License - Identifier: GPL - 3.0 +
+import unittest
+
+from mantid.py3compat import mock
+from plugins.algorithms.WorkflowAlgorithms.SANS.SANSBeamCentreFinder import SANSBeamCentreFinder, _ResidualsDetails
+
+
+class SANSBeamCentreFinderTest(unittest.TestCase):
+    @staticmethod
+    def gen_mock_data(q_vals, y_vals):
+        assert len(q_vals) == len(y_vals)
+        mocked_quartile = mock.Mock()
+        mocked_quartile.readX.return_value = q_vals
+        mocked_quartile.readY.return_value = y_vals
+        return mocked_quartile
+
+    def test_points_calculates_correctly(self):
+        points_1 = [1.0, 2.0, 4.0]
+        points_2 = [1.0, 3.0, 5.0]
+
+        q1_data = self.gen_mock_data(points_1, [0., 0., 0.])
+        q2_data = self.gen_mock_data(points_2, [0., 0., 0.])
+
+        expected_points = len(set().union(points_1, points_2))
+        mismatched_points = len(set(points_1).symmetric_difference(set(points_2)))
+
+        obj = SANSBeamCentreFinder()
+        result = obj._calculate_residuals(q1_data, q2_data)
+        self.assertIsInstance(result, _ResidualsDetails)
+        self.assertEqual(mismatched_points, result.mismatched_points)
+        self.assertEqual(expected_points, result.num_points_considered)
+
+    def test_residual_diff_with_only_mismatched(self):
+        q1_data = self.gen_mock_data([1.0, 3.0], [0.0, 1.0])
+        q2_data = self.gen_mock_data([1.0, 2.0], [0.0, 1.0])
+
+        # Mismatched points should always contribute their entire val, not the diff (i.e. 1. above)
+        obj = SANSBeamCentreFinder()
+        result = obj._calculate_residuals(q1_data, q2_data)
+        self.assertEqual(2.0, result.total_residual)
+
+    def test_residual_diff_with_only_matched(self):
+        y_data_1 = [1.0, 10.0]
+        y_data_2 = [2.0, 10.0]
+        q1_data = self.gen_mock_data([1.0, 2.0], y_data_1)
+        q2_data = self.gen_mock_data([1.0, 2.0], y_data_2)
+
+        expected_matched = (y_data_1[0] - y_data_2[0]) ** 2
+
+        obj = SANSBeamCentreFinder()
+        result = obj._calculate_residuals(q1_data, q2_data)
+
+        self.assertEqual(expected_matched, result.total_residual)
+
+    def test_residual_diff_with_mixed_mismatched(self):
+        y_data_1 = [1.0, 10.0]
+        y_data_2 = [2.0, 10.0]
+        q1_data = self.gen_mock_data([1.0, 3.0], y_data_1)
+        q2_data = self.gen_mock_data([1.0, 2.0], y_data_2)
+
+        obj = SANSBeamCentreFinder()
+        result = obj._calculate_residuals(q1_data, q2_data)
+
+        # Matched bins contribute the diff ([0]) whilst mismatched ([1]) contribute all
+        expected_matched = (y_data_1[0] - y_data_2[0]) ** 2
+        expected_mismatched = (y_data_1[1] ** 2) + (y_data_2[1] ** 2)
+        squared_expected = expected_matched + expected_mismatched
+        self.assertEqual(squared_expected, result.total_residual)
+
+
+if __name__ == '__main__':
+    unittest.main()

docs/source/release/v4.3.0/sans.rst

@@ -42,5 +42,9 @@ Fixed
 - A zero monitor shift did not previously account for the position
   of the rear detector for Zoom. A 0.0mm offset now works correctly when
   processing data from the SANS GUI or command interface.
+- The Beam Centre Finder will now prints the average error in X, Y and the
+  number of points considered. This allows for direct comparisons of different
+  radius limits, which previously summed the difference across different
+  numbers of points.
 
 :ref:`Release 4.3.0 <v4.3.0>`

qt/python/mantidqt/plotting/test/test_tiledplots.py

@@ -93,7 +93,7 @@ class TiledPlotsTest(TestCase):
         self.assertEqual(fig, initial_fig)
         self.assertEqual(6, len(fig.axes))
 
-    def test_get_plot_fig_with_no_figure_provided_and_overplot_is_true_returns_current_figure_unmodified(self):
+    def test_get_plot_fig_with_no_figure_provided_and_overplot_is_true_returns_new_figure(self):
         initial_fig = plt.Figure()
         set_figure_as_current_figure(initial_fig)