Refs #17756 Rosace detector IDs now set correctly

Removed some code duplication that led to this bug. Also added a sanity check on the detector IDs, to check none have an invalid value.

Framework/DataHandling/inc/MantidDataHandling/LoadILLTOF.h

@@ -7,6 +7,8 @@
 #include "MantidAPI/IFileLoader.h"
 #include "MantidNexus/NexusClasses.h"
 #include "MantidDataHandling/LoadHelper.h"
+#include "MantidGeometry/IDTypes.h"
+#include "MantidAPI/Progress.h"
 
 namespace Mantid {
 namespace DataHandling {
@@ -75,6 +77,10 @@ private:
   loadDataIntoTheWorkSpace(NeXus::NXEntry &entry,
                            const std::vector<std::vector<int>> &,
                            int vanaCalculatedDetectorElasticPeakPosition = -1);
+  void loadSpectra(size_t &spec, size_t firstSpec, size_t numberOfMonitors,
+                   size_t numberOfTubes,
+                   std::vector<Mantid::detid_t> &detectorIDs, NeXus::NXInt data,
+                   Mantid::API::Progress progress);
 
   void runLoadInstrument();
 

Framework/DataHandling/src/LoadILLTOF.cpp

@@ -531,26 +531,8 @@ void LoadILLTOF::loadDataIntoTheWorkSpace(
       .assign(detectorTofBins.begin(), detectorTofBins.end());
 
   Progress progress(this, 0, 1, m_numberOfTubes * m_numberOfPixelsPerTube);
-  for (size_t i = 0; i < m_numberOfTubes; ++i) {
-    for (size_t j = 0; j < m_numberOfPixelsPerTube; ++j) {
-      if (spec > firstSpec) {
-        // just copy the time binning axis to every spectra
-        m_localWorkspace->dataX(spec) = m_localWorkspace->readX(firstSpec);
-      }
-      // Assign Y
-      int *data_p = &data(static_cast<int>(i), static_cast<int>(j), 0);
-      m_localWorkspace->dataY(spec).assign(data_p, data_p + m_numberOfChannels);
-
-      // Assign Error
-      MantidVec &E = m_localWorkspace->dataE(spec);
-      std::transform(data_p, data_p + m_numberOfChannels, E.begin(),
-                     LoadILLTOF::calculateError);
-      m_localWorkspace->getSpectrum(spec)
-          .setDetectorID(detectorIDs[spec - numberOfMonitors]);
-      ++spec;
-      progress.report();
-    }
-  }
+  loadSpectra(spec, firstSpec, numberOfMonitors, m_numberOfTubes, detectorIDs,
+              data, progress);
 
   g_log.debug() << "Loading data into the workspace: DONE!\n";
 
@@ -569,24 +551,46 @@ void LoadILLTOF::loadDataIntoTheWorkSpace(
 
     Progress progressRosace(this, 0, 1,
                             numberOfTubes * m_numberOfPixelsPerTube);
-    for (size_t i = 0; i < numberOfTubes; ++i) {
-      for (size_t j = 0; j < m_numberOfPixelsPerTube; ++j) {
-        // just copy the time binning axis to every spectra
-        m_localWorkspace->dataX(spec) = m_localWorkspace->readX(firstSpec);
-
-        // Assign Y
-        int *data_p = &dataRosace(static_cast<int>(i), static_cast<int>(j), 0);
-        m_localWorkspace->dataY(spec)
-            .assign(data_p, data_p + m_numberOfChannels);
 
-        // Assign Error
-        MantidVec &E = m_localWorkspace->dataE(spec);
-        std::transform(data_p, data_p + m_numberOfChannels, E.begin(),
-                       LoadILLTOF::calculateError);
+    loadSpectra(spec, firstSpec, numberOfMonitors, numberOfTubes, detectorIDs,
+                dataRosace, progressRosace);
+  }
+}
 
-        ++spec;
-        progressRosace.report();
+/**
+ * Loops over all the pixels and loads the correct spectra. Called for each set
+ * of detector types in the workspace.
+ *
+ * @param spec The current spectrum id
+ * @param firstSpec The id of the first spectrum that is not a monitor
+ * @param numberOfMonitors The number of monitors in the workspace
+ * @param numberOfTubes The number of detector tubes in the workspace
+ * @param detectorIDs A list of all of the detector IDs
+ * @param data The NeXus data to load into the workspace
+ * @param progress The progress monitor (different
+ */
+void LoadILLTOF::loadSpectra(size_t &spec, size_t firstSpec,
+                             size_t numberOfMonitors, size_t numberOfTubes,
+                             std::vector<detid_t> &detectorIDs, NXInt data,
+                             Progress progress) {
+  for (size_t i = 0; i < numberOfTubes; ++i) {
+    for (size_t j = 0; j < m_numberOfPixelsPerTube; ++j) {
+      if (spec > firstSpec) {
+        // just copy the time binning axis to every spectra
+        m_localWorkspace->dataX(spec) = m_localWorkspace->readX(firstSpec);
       }
+      // Assign Y
+      int *data_p = &data(static_cast<int>(i), static_cast<int>(j), 0);
+      m_localWorkspace->dataY(spec).assign(data_p, data_p + m_numberOfChannels);
+
+      // Assign Error
+      MantidVec &E = m_localWorkspace->dataE(spec);
+      std::transform(data_p, data_p + m_numberOfChannels, E.begin(),
+                     LoadILLTOF::calculateError);
+      m_localWorkspace->getSpectrum(spec)
+          .setDetectorID(detectorIDs[spec - numberOfMonitors]);
+      ++spec;
+      progress.report();
     }
   }
 }

Framework/DataHandling/test/LoadILLTOFTest.h

@@ -53,6 +53,15 @@ public:
 
     TS_ASSERT_EQUALS(output2D->getNumberHistograms(), numberOfHistograms);
 
+    // Check all detectors have a unique and defined detector ID >= 0
+
+    Mantid::detid2index_map detectorMap;
+    TS_ASSERT_THROWS_NOTHING(detectorMap = output->getDetectorIDToWorkspaceIndexMap(true));
+
+    for (auto value : detectorMap){
+      TS_ASSERT(value.first >= 0);
+    }
+
     AnalysisDataService::Instance().clear();
   }