LoadEventAsWorkspace2D: consistent duration when using time filtering (#40963)

The calculation of the log `duration` when using time filtering was
found to be inconsistent between `LoadEventNexus` and
`LoadEventAsWorkspace2D`. This PR updates `LoadEventAsWorkspace2D` to be
consistent with `LoadEventNexus` and update the `duration` to the time
between `FilterByTimeStart` and `FilterByTimeStop`.

ORNL issue: [Defect
15001](https://ornlrse.clm.ibmcloud.com/ccm/web/projects/Neutron%20Data%20Project%20(Change%20Management)#action=com.ibm.team.workitem.viewWorkItem&id=15001)

Author: backmari

Framework/DataHandling/src/LoadEventAsWorkspace2D.cpp

@@ -237,6 +237,18 @@ void LoadEventAsWorkspace2D::exec() {
   Types::Core::DateAndTime filter_time_start;
   Types::Core::DateAndTime filter_time_stop;
 
+  // compute time filter bounds
+  if (filter_time_start_sec != EMPTY_DBL()) {
+    filter_time_start = runstart + filter_time_start_sec;
+  } else {
+    filter_time_start = runstart;
+  }
+  if (filter_time_stop_sec != EMPTY_DBL()) {
+    filter_time_stop = runstart + filter_time_stop_sec;
+  } else {
+    filter_time_stop = endtime;
+  }
+
   // vector to stored to integrated counts by detector ID
   std::vector<uint32_t> Y(max_detid - min_detid + 1, 0);
 
@@ -245,7 +257,7 @@ void LoadEventAsWorkspace2D::exec() {
   h5file.openAddress("/");
   h5file.openGroup("entry", "NXentry");
 
-  // Now we want to go through all the bankN_event entrie
+  // Now we want to go through all the bankN_event entries
   prog->doReport("Reading and integrating data");
 
   std::set<std::string> const classEntries = h5file.getEntriesByClass("NXevent_data");
@@ -282,25 +294,11 @@ void LoadEventAsWorkspace2D::exec() {
         const auto event_index =
             std::make_shared<std::vector<uint64_t>>(Nexus::IOHelper::readNexusVector<uint64_t>(h5file, "event_index"));
 
-        // if "filterTimeStart" is empty, use run start time as default
-        if (filter_time_start_sec != EMPTY_DBL()) {
-          filter_time_start = runstart + filter_time_start_sec;
-        } else {
-          filter_time_start = runstart;
-        }
+        // Use filter_time_start time as starting reference in time and create a pulseROI using bankPulseTimes
+        const auto pulseROI = bankPulseTimes->getPulseIndices(filter_time_start, filter_time_stop);
 
-        // if "filterTimeStop" is empty, use end time as default
-        if (filter_time_stop_sec != EMPTY_DBL()) {
-          filter_time_stop = runstart + filter_time_stop_sec;
-        } else {
-          filter_time_stop = endtime;
-        }
-
-        // Use filter_time_start time as starting reference in time and create a TimeROI using bankPulseTimes
-        const auto TimeROI = bankPulseTimes->getPulseIndices(filter_time_start, filter_time_stop);
-
-        // set up PulseIndexer and give previous TimeROI to pulseIndexer
-        const PulseIndexer pulseIndexer(event_index, event_index->at(0), event_ids.size(), entry_name, TimeROI);
+        // set up PulseIndexer and give previous pulseROI to pulseIndexer
+        const PulseIndexer pulseIndexer(event_index, event_index->at(0), event_ids.size(), entry_name, pulseROI);
 
         std::vector<float> event_times;
         if (tof_filtering) {
@@ -310,7 +308,7 @@ void LoadEventAsWorkspace2D::exec() {
             event_times = Nexus::IOHelper::readNexusVector<float>(h5file, "event_time_of_flight");
         }
 
-        // Nested loop to loop through all the relavent pulses and relavent event_ids.
+        // Nested loop to loop through all the relevant pulses and relevant event_ids.
         // For someone new to this, every pulse creates an entry in event_index, event_index.size() = # of pulses,
         // the value of event_index[i] points to the index of event_ids. In short, event_ids[event_index[i]] is the
         // detector id from the i-th pulse. See NXevent_data description for more details.
@@ -336,6 +334,14 @@ void LoadEventAsWorkspace2D::exec() {
   h5file.closeGroup();
   h5file.close();
 
+  // filter the logs the same way FilterByTime does
+  const bool is_time_filtered = (filter_time_start_sec != EMPTY_DBL() || filter_time_stop_sec != EMPTY_DBL());
+  if (is_time_filtered) {
+    Kernel::TimeROI timeroi(filter_time_start, filter_time_stop);
+    outWS->mutableRun().setTimeROI(timeroi);
+    outWS->mutableRun().removeDataOutsideTimeROI();
+  }
+
   // determine x values
   const auto xBins = {center - width / 2, center + width / 2};
 

Framework/DataHandling/src/PulseIndexer.cpp

@@ -53,10 +53,12 @@ PulseIndexer::PulseIndexer(std::shared_ptr<std::vector<uint64_t> const> const &e
 
   // determine if should trim the back end to remove empty pulses
   auto lastPulseIndex = m_roi.back();
-  eventRange = this->getEventIndexRange(lastPulseIndex - 1);
-  while (eventRange.first == eventRange.second && eventRange.second > 0) {
-    --lastPulseIndex;
+  if (lastPulseIndex > 0) {
     eventRange = this->getEventIndexRange(lastPulseIndex - 1);
+    while (eventRange.first == eventRange.second && eventRange.second > 0 && lastPulseIndex > 1) {
+      --lastPulseIndex;
+      eventRange = this->getEventIndexRange(lastPulseIndex - 1);
+    }
   }
 
   // update the value if it has changed

Framework/DataHandling/test/LoadEventAsWorkspace2DTest.h

@@ -11,6 +11,7 @@
 #include "MantidAPI/AlgorithmManager.h"
 #include "MantidAPI/Axis.h"
 #include "MantidAPI/FileFinder.h"
+#include "MantidAPI/Run.h"
 #include "MantidDataHandling/LoadEventAsWorkspace2D.h"
 #include "MantidDataObjects/EventWorkspace.h"
 #include "MantidDataObjects/Workspace2D.h"
@@ -269,6 +270,13 @@ class LoadEventAsWorkspace2DTest : public CxxTest::TestSuite {
     compare->setProperty("Workspace2", outputWS3);
     compare->execute();
     TS_ASSERT(compare->getProperty("Result"));
+
+    // compare the duration, which should have been filtered to 5s
+    constexpr auto expected_duration = 5.0;
+    const auto duration1 = outputWS->run().getPropertyValueAsType<double>("duration");
+    const auto duration2 = outputWS3->run().getPropertyValueAsType<double>("duration");
+    TS_ASSERT_DELTA(duration1, expected_duration, 1e-6);
+    TS_ASSERT_DELTA(duration2, expected_duration, 1e-6);
   }
 
   void test_BSS_filterbytimeStart() {

Framework/DataHandling/test/PulseIndexerTest.h

@@ -291,4 +291,32 @@ class PulseIndexerTest : public CxxTest::TestSuite {
       TS_ASSERT_EQUALS(num_steps, 2); // calculated by hand
     }
   }
+
+  // regression test: constructing with a pulse_roi that contains no events must not infinite-loop
+  void test_noEventsInPulseROI() {
+    // event_index where pulses 1 and 2 are empty (repeated value 10)
+    auto eventIndices = std::make_shared<std::vector<uint64_t>>();
+    eventIndices->push_back(0);
+    eventIndices->push_back(10);
+    eventIndices->push_back(10); // pulse 1: empty
+    eventIndices->push_back(10); // pulse 2: empty
+    eventIndices->push_back(20);
+
+    constexpr size_t start_event_index{0};
+    constexpr size_t total_events{20};
+
+    // roi selects only pulses 1 and 2, both of which have no events
+    std::vector<size_t> roi{1, 3};
+    PulseIndexer indexer(eventIndices, start_event_index, total_events, entry_name, roi);
+
+    // the roi contains no events, so the indexer should produce an empty range
+    TS_ASSERT_EQUALS(indexer.getFirstPulseIndex(), indexer.getLastPulseIndex());
+
+    size_t num_steps{0};
+    for (const auto &iter : indexer) {
+      (void)iter;
+      num_steps++;
+    }
+    TS_ASSERT_EQUALS(num_steps, 0);
+  }
 };

docs/source/release/v6.16.0/Framework/Algorithms/Bugfixes/40963.rst

@@ -0,0 +1,6 @@
+- Algorithm :ref:`LoadEventAsWorkspace2D <algm-LoadEventAsWorkspace2D>` now updates the run duration log when using
+  parameters ``FilterByTimeStart`` and/or ``FilterByTimeStop``. The behaviour is now consistent with
+  :ref:`LoadEventNexus <algm-LoadEventNexus>` when using the same parameters.
+- Loading with :ref:`LoadEventAsWorkspace2D <algm-LoadEventAsWorkspace2D>` and
+  :ref:`LoadEventNexus <algm-LoadEventNexus>` with time filtering when there are banks with no events in that time
+  range no longer causes the loading to hang due to an infinite loop.