Add optional IncludeDetectorPosition to CreateDetectorTable (#38960)

* optional add position flag for detector table

* updated new parameter declaration

* added detector position to exception handling

* updated the docstring of createDetectorTableWorkspace

* added release note

* disable include position option for peaks ws

* added test coverage for the no detector exceptions

* add semicolons to endo of assert statements

* check table population isn't just null values

Author: andy-bridger

Framework/Algorithms/inc/MantidAlgorithms/CreateDetectorTable.h

@@ -54,7 +54,7 @@ class MANTID_ALGORITHMS_DLL CreateDetectorTable final : public API::Algorithm {
 /// Creates table workspace of detector information from a given workspace
 API::ITableWorkspace_sptr createDetectorTableWorkspace(const API::MatrixWorkspace_sptr &ws,
                                                        const std::vector<int> &indices, const bool includeData,
-                                                       Kernel::Logger &logger);
+                                                       const bool includeDetectorPosition, Kernel::Logger &logger);
 
 /// Converts a list to a string, shortened if necessary
 std::string createTruncatedList(const std::set<int> &elements);
@@ -63,9 +63,11 @@ void populateTable(Mantid::API::ITableWorkspace_sptr &t, const Mantid::API::Matr
                    const std::vector<int> &indices, const Mantid::API::SpectrumInfo &spectrumInfo,
                    bool signedThetaParamRetrieved, bool showSignedTwoTheta,
                    const Mantid::Geometry::PointingAlong &beamAxisIndex, const double sampleDist, const bool isScanning,
-                   const bool include_data, const bool calcQ, const bool includeDiffConstants, Kernel::Logger &logger);
+                   const bool include_data, const bool calcQ, const bool includeDiffConstants,
+                   const bool includeDetectorPosition, Kernel::Logger &logger);
 std::vector<std::pair<std::string, std::string>> createColumns(const bool isScanning, const bool includeData,
-                                                               const bool calcQ, const bool hasDiffConstants);
+                                                               const bool calcQ, const bool hasDiffConstants,
+                                                               const bool includeDetectorPosition);
 
 } // namespace Algorithms
 } // namespace Mantid

Framework/Algorithms/src/CreateDetectorTable.cpp

@@ -27,7 +27,10 @@ void CreateDetectorTable::init() {
   declareProperty("IncludeData", false, "Include the first value from each spectrum.");
   setPropertySettings("IncludeData",
                       std::make_unique<EnabledWhenWorkspaceIsType<MatrixWorkspace>>("InputWorkspace", true));
-
+  declareProperty<bool>("IncludeDetectorPosition", false,
+                        "Include the absolute position of the detector group for each spectrum.", Direction::Input);
+  setPropertySettings("IncludeDetectorPosition",
+                      std::make_unique<EnabledWhenWorkspaceIsType<MatrixWorkspace>>("InputWorkspace", true));
   declareProperty(std::make_unique<WorkspaceProperty<TableWorkspace>>("DetectorTableWorkspace", "", Direction::Output,
                                                                       PropertyMode::Optional),
                   "The name of the outputted detector table workspace, if left empty then "
@@ -38,6 +41,7 @@ void CreateDetectorTable::exec() {
   Workspace_sptr inputWS = getProperty("InputWorkspace");
   bool includeData = getProperty("IncludeData");
   std::vector<int> indices = getProperty("WorkspaceIndices");
+  bool includeDetectorPosition = getProperty("IncludeDetectorPosition");
 
   ITableWorkspace_sptr detectorTable;
 
@@ -46,7 +50,7 @@ void CreateDetectorTable::exec() {
     if (sample == nullptr) {
       throw std::runtime_error("Matrix workspace has no instrument information");
     }
-    detectorTable = createDetectorTableWorkspace(matrix, indices, includeData, g_log);
+    detectorTable = createDetectorTableWorkspace(matrix, indices, includeData, includeDetectorPosition, g_log);
 
     if (detectorTable == nullptr) {
       throw std::runtime_error("Unknown error while creating detector table for matrix workspace");
@@ -98,12 +102,15 @@ std::map<std::string, std::string> CreateDetectorTable::validateInputs() {
  * @param indices :: Limit the table to these workspace indices
  * @param includeData :: If true then first value from the each spectrum is
  * displayed
+ * @param includeDetectorPosition :: If true then include the absolute position of
+ * the detector group for each spectrum
  * @param logger: The Mantid logger so errors can be written to it.
  *
  * @return A pointer to the table workspace of detector information
  */
 ITableWorkspace_sptr createDetectorTableWorkspace(const MatrixWorkspace_sptr &ws, const std::vector<int> &indices,
-                                                  const bool includeData, Logger &logger) {
+                                                  const bool includeData, const bool includeDetectorPosition,
+                                                  Logger &logger) {
   IComponent_const_sptr sample = ws->getInstrument()->getSample();
 
   // check if efixed value is available
@@ -134,7 +141,7 @@ ITableWorkspace_sptr createDetectorTableWorkspace(const MatrixWorkspace_sptr &ws
   }
 
   // Prepare column names
-  auto colNames = createColumns(isScanning, includeData, calcQ, hasDiffConstants);
+  auto colNames = createColumns(isScanning, includeData, calcQ, hasDiffConstants, includeDetectorPosition);
 
   const int ncols = static_cast<int>(colNames.size());
   const int nrows = indices.empty() ? static_cast<int>(ws->getNumberHistograms()) : static_cast<int>(indices.size());
@@ -156,13 +163,14 @@ ITableWorkspace_sptr createDetectorTableWorkspace(const MatrixWorkspace_sptr &ws
                                                                     // value should be displayed
 
   populateTable(t, ws, nrows, indices, spectrumInfo, signedThetaParamRetrieved, showSignedTwoTheta, beamAxisIndex,
-                sampleDist, isScanning, includeData, calcQ, hasDiffConstants, logger);
+                sampleDist, isScanning, includeData, calcQ, hasDiffConstants, includeDetectorPosition, logger);
 
   return t;
 }
 
 std::vector<std::pair<std::string, std::string>> createColumns(const bool isScanning, const bool includeData,
-                                                               const bool calcQ, const bool hasDiffConstants) {
+                                                               const bool calcQ, const bool hasDiffConstants,
+                                                               const bool includeDetectorPosition) {
   std::vector<std::pair<std::string, std::string>> colNames;
   colNames.emplace_back("double", "Index");
   colNames.emplace_back("int", "Spectrum No");
@@ -187,14 +195,17 @@ std::vector<std::pair<std::string, std::string>> createColumns(const bool isScan
     colNames.emplace_back("double", "DIFC - Uncalibrated");
     colNames.emplace_back("double", "TZERO");
   }
+  if (includeDetectorPosition) {
+    colNames.emplace_back("V3D", "Position");
+  }
   return colNames;
 }
 
 void populateTable(ITableWorkspace_sptr &t, const MatrixWorkspace_sptr &ws, const int nrows,
                    const std::vector<int> &indices, const SpectrumInfo &spectrumInfo, bool signedThetaParamRetrieved,
                    bool showSignedTwoTheta, const PointingAlong &beamAxisIndex, const double sampleDist,
                    const bool isScanning, const bool includeData, const bool calcQ, const bool includeDiffConstants,
-                   Logger &logger) {
+                   const bool includeDetectorPosition, Logger &logger) {
   PARALLEL_FOR_IF(Mantid::Kernel::threadSafe(*ws))
   for (int row = 0; row < nrows; ++row) {
     TableRow colValues = t->getRow(row);
@@ -299,6 +310,10 @@ void populateTable(ITableWorkspace_sptr &t, const MatrixWorkspace_sptr &ws, cons
                     << diffConsts[UnitParams::tzero];
         }
       }
+      if (includeDetectorPosition) {
+        const auto &detectorPosition = spectrumInfo.position(wsIndex);
+        colValues << detectorPosition;
+      }
     } catch (const std::exception &) {
       colValues.row(row);
       colValues << static_cast<double>(wsIndex);
@@ -317,6 +332,9 @@ void populateTable(ITableWorkspace_sptr &t, const MatrixWorkspace_sptr &ws, cons
       if (includeDiffConstants) {
         colValues << 0.0 << 0.0 << 0.0 << 0.0;
       }
+      if (includeDetectorPosition) {
+        colValues << V3D(0.0, 0.0, 0.0);
+      }
     } // End catch for no spectrum
   }
 }

Framework/Algorithms/test/CreateDetectorTableTest.h

@@ -41,7 +41,7 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     TS_ASSERT(alg.isInitialized());
 
     const auto &props = alg.getProperties();
-    TS_ASSERT_EQUALS(props.size(), 4);
+    TS_ASSERT_EQUALS(props.size(), 5);
 
     TS_ASSERT_EQUALS(props[0]->name(), "InputWorkspace");
     TS_ASSERT(props[0]->isDefault());
@@ -52,17 +52,20 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     TS_ASSERT_EQUALS(props[2]->name(), "IncludeData");
     TS_ASSERT(props[2]->isDefault());
 
-    TS_ASSERT_EQUALS(props[3]->name(), "DetectorTableWorkspace");
+    TS_ASSERT_EQUALS(props[3]->name(), "IncludeDetectorPosition");
     TS_ASSERT(props[3]->isDefault());
-    TS_ASSERT(dynamic_cast<WorkspaceProperty<TableWorkspace> *>(props[3]));
+
+    TS_ASSERT_EQUALS(props[4]->name(), "DetectorTableWorkspace");
+    TS_ASSERT(props[4]->isDefault());
+    TS_ASSERT(dynamic_cast<WorkspaceProperty<TableWorkspace> *>(props[4]));
   }
 
   void test_Exec_Matrix_Workspace() {
     Workspace2D_sptr inputWS = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(2, 10);
 
     CreateDetectorTable alg;
-    TS_ASSERT_THROWS_NOTHING(alg.initialize())
-    TS_ASSERT(alg.isInitialized())
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
     TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", std::dynamic_pointer_cast<MatrixWorkspace>(inputWS)));
     TS_ASSERT_THROWS_NOTHING(alg.execute();)
     TS_ASSERT(alg.isExecuted());
@@ -81,6 +84,39 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     // Check the results
     TS_ASSERT_EQUALS(ws->columnCount(), 11);
     TS_ASSERT_EQUALS(ws->rowCount(), 2);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), 1); // Spectrum No should be 1, if not in the exception
+
+    // Remove workspace from the data service.
+    AnalysisDataService::Instance().remove(ws->getName());
+  }
+
+  void test_Exec_Matrix_Workspace_with_no_valid_spectra() {
+    Workspace2D_sptr inputWS = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(1, 10);
+    auto &spec = inputWS->getSpectrum(0);
+    spec.clearDetectorIDs(); // clear the detectors to test the exception catching
+
+    CreateDetectorTable alg;
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", std::dynamic_pointer_cast<MatrixWorkspace>(inputWS)));
+    TS_ASSERT_THROWS_NOTHING(alg.execute();)
+    TS_ASSERT(alg.isExecuted());
+
+    // Not setting an output workspace name should give the name:
+    //[input workspace name] + "-Detectors"
+    TableWorkspace_sptr ws;
+    TS_ASSERT_THROWS_NOTHING(
+        ws = AnalysisDataService::Instance().retrieveWS<TableWorkspace>(inputWS->getName() + "-Detectors"));
+    TS_ASSERT(ws);
+
+    if (!ws) {
+      return;
+    }
+
+    // Check the results
+    TS_ASSERT_EQUALS(ws->columnCount(), 11);
+    TS_ASSERT_EQUALS(ws->rowCount(), 1);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), -1); // Spectrum No should be -1
 
     // Remove workspace from the data service.
     AnalysisDataService::Instance().remove(ws->getName());
@@ -91,8 +127,8 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     std::string outWSName{"Detector Table Test"};
 
     CreateDetectorTable alg;
-    TS_ASSERT_THROWS_NOTHING(alg.initialize())
-    TS_ASSERT(alg.isInitialized())
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
     TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", std::dynamic_pointer_cast<MatrixWorkspace>(inputWS)));
     TS_ASSERT_THROWS_NOTHING(alg.setProperty("WorkspaceIndices", "1"));
     TS_ASSERT_THROWS_NOTHING(alg.setProperty("IncludeData", true));
@@ -122,6 +158,7 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     // Check the results
     TS_ASSERT_EQUALS(ws->columnCount(), 13);
     TS_ASSERT_EQUALS(ws->rowCount(), 1);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), 2); // Spectrum No should be 2 due to the WorkspaceIndex
 
     // Remove workspace from the data service.
     AnalysisDataService::Instance().remove(outWSName);
@@ -131,11 +168,11 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     PeaksWorkspace_sptr inputWS = WorkspaceCreationHelper::createPeaksWorkspace(5, false);
 
     CreateDetectorTable alg;
-    TS_ASSERT_THROWS_NOTHING(alg.initialize())
-    TS_ASSERT(alg.isInitialized())
-    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", inputWS))
-    TS_ASSERT_THROWS_NOTHING(alg.execute())
-    TS_ASSERT(alg.isExecuted())
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", inputWS));
+    TS_ASSERT_THROWS_NOTHING(alg.execute());
+    TS_ASSERT(alg.isExecuted());
 
     TableWorkspace_sptr ws;
     TS_ASSERT_THROWS_NOTHING(
@@ -147,8 +184,9 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     }
 
     // Check the results
-    TS_ASSERT_EQUALS(ws->columnCount(), 2)
-    TS_ASSERT_EQUALS(ws->rowCount(), 5)
+    TS_ASSERT_EQUALS(ws->columnCount(), 2);
+    TS_ASSERT_EQUALS(ws->rowCount(), 5);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), 0); // First column is Index when exec on PeaksWorkspace, so expect 0
 
     // Remove workspace from the data service.
     AnalysisDataService::Instance().remove(ws->getName());
@@ -158,12 +196,79 @@ class CreateDetectorTableTest : public CxxTest::TestSuite {
     ITableWorkspace_sptr inputWS = std::make_shared<TableWorkspace>();
 
     CreateDetectorTable alg;
-    TS_ASSERT_THROWS_NOTHING(alg.initialize())
-    TS_ASSERT(alg.isInitialized())
-    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", inputWS))
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", inputWS));
 
     TS_ASSERT_THROWS(alg.executeAsChildAlg(), const std::runtime_error &);
   }
+
+  void test_Exec_Matrix_Workspace_with_Include_DetPos() {
+    Workspace2D_sptr inputWS = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(3, 10, true);
+
+    CreateDetectorTable alg;
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", std::dynamic_pointer_cast<MatrixWorkspace>(inputWS)));
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("IncludeDetectorPosition", true));
+    TS_ASSERT_THROWS_NOTHING(alg.execute());
+    TS_ASSERT(alg.isExecuted());
+
+    // Not setting an output workspace name should give the name:
+    //[input workspace name] + "-Detectors"
+    TableWorkspace_sptr ws;
+    TS_ASSERT_THROWS_NOTHING(
+        ws = AnalysisDataService::Instance().retrieveWS<TableWorkspace>(inputWS->getName() + "-Detectors"));
+    TS_ASSERT(ws);
+
+    if (!ws) {
+      return;
+    }
+
+    // Check the results
+    TS_ASSERT_EQUALS(ws->columnCount(), 12); // extra column compared to test_Exec_Matrix_Workspace
+    TS_ASSERT_EQUALS(ws->rowCount(), 3);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), 1); // Spectrum No should be 1, if not in the exception
+    TS_ASSERT_EQUALS(ws->cell<V3D>(1, 11),
+                     V3D(0.0, 0.0, -9.0)); // Last two are monitors, first position should be (0.0, 0.0, -9.0)
+
+    // Remove workspace from the data service.
+    AnalysisDataService::Instance().remove(ws->getName());
+  }
+
+  void test_Exec_Matrix_Workspace_with_no_valid_spectra_include_DetPos() {
+    Workspace2D_sptr inputWS = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(1, 10);
+    auto &spec = inputWS->getSpectrum(0);
+    spec.clearDetectorIDs(); // clear the detectors to test the exception catching
+
+    CreateDetectorTable alg;
+    TS_ASSERT_THROWS_NOTHING(alg.initialize());
+    TS_ASSERT(alg.isInitialized());
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("InputWorkspace", std::dynamic_pointer_cast<MatrixWorkspace>(inputWS)));
+    TS_ASSERT_THROWS_NOTHING(alg.setProperty("IncludeDetectorPosition", true));
+    TS_ASSERT_THROWS_NOTHING(alg.execute());
+    TS_ASSERT(alg.isExecuted());
+
+    // Not setting an output workspace name should give the name:
+    //[input workspace name] + "-Detectors"
+    TableWorkspace_sptr ws;
+    TS_ASSERT_THROWS_NOTHING(
+        ws = AnalysisDataService::Instance().retrieveWS<TableWorkspace>(inputWS->getName() + "-Detectors"));
+    TS_ASSERT(ws);
+
+    if (!ws) {
+      return;
+    }
+
+    // Check the results
+    TS_ASSERT_EQUALS(ws->columnCount(), 12);
+    TS_ASSERT_EQUALS(ws->rowCount(), 1);
+    TS_ASSERT_EQUALS(ws->cell<int>(0, 1), -1);                  // Spectrum No should be -1
+    TS_ASSERT_EQUALS(ws->cell<V3D>(0, 11), V3D(0.0, 0.0, 0.0)); // Detector Position should be (0.0, 0.0, 0.0)
+
+    // Remove workspace from the data service.
+    AnalysisDataService::Instance().remove(ws->getName());
+  }
 };
 
 class CreateDetectorTablePerformance : public CxxTest::TestSuite {

docs/source/release/v6.13.0/Framework/Algorithms/New_features/38960.rst

@@ -0,0 +1 @@
+- :ref:`CreateDetectorTable<algm-CreateDetectorTable>` now has an optional argument, ``IncludeDetectorPosition``, which, if `True`, will add a column, ``Position``, to the end of the ``DetectorTable``. This column has the detector position as a ``V3D`` type object for each spectrum in the table.