From 063c7ff151a1a7d2122a6d5ccf3b955d76e97ec0 Mon Sep 17 00:00:00 2001
From: Samuel Jackson <samueljackson@outlook.com>
Date: Mon, 8 Oct 2018 17:31:24 +0100
Subject: [PATCH] Refs #23666 Fix goniometer using mean instead of time average
---
Framework/API/src/Run.cpp | 14 ++--
Framework/DataHandling/src/LoadNexusLogs.cpp | 72 ++++++++++++++++---
.../DataHandling/test/LoadISISNexusTest.h | 4 +-
.../DataHandling/test/LoadNexusLogsTest.h | 25 ++++++-
docs/source/release/v3.14.0/framework.rst | 1 +
.../samplelogs/test/test_samplelogs_model.py | 2 +-
6 files changed, 97 insertions(+), 21 deletions(-)
diff --git a/Framework/API/src/Run.cpp b/Framework/API/src/Run.cpp
index 6486aec0feb..b91e5f6b86f 100644
--- a/Framework/API/src/Run.cpp
+++ b/Framework/API/src/Run.cpp
@@ -468,16 +468,18 @@ void Run::calculateGoniometerMatrix() {
getLogAsSingleValue(axisName, Kernel::Math::Minimum);
const double maxAngle =
getLogAsSingleValue(axisName, Kernel::Math::Maximum);
- const double angle = getLogAsSingleValue(axisName, Kernel::Math::Mean);
+ const double angle =
+ getLogAsSingleValue(axisName, Kernel::Math::TimeAveragedMean);
+
if (minAngle != maxAngle &&
!(std::isnan(minAngle) && std::isnan(maxAngle))) {
const double lastAngle =
getLogAsSingleValue(axisName, Kernel::Math::LastValue);
- g_log.warning(
- "Goniometer angle changed in " + axisName + " log from " +
- boost::lexical_cast<std::string>(minAngle) + " to " +
- boost::lexical_cast<std::string>(maxAngle) +
- ". Used mean = " + boost::lexical_cast<std::string>(angle) + ".");
+ g_log.warning("Goniometer angle changed in " + axisName + " log from " +
+ boost::lexical_cast<std::string>(minAngle) + " to " +
+ boost::lexical_cast<std::string>(maxAngle) +
+ ". Used time averaged value = " +
+ boost::lexical_cast<std::string>(angle) + ".");
if (axisName == "omega") {
g_log.warning("To set to last angle, replace omega with " +
boost::lexical_cast<std::string>(lastAngle) +
diff --git a/Framework/DataHandling/src/LoadNexusLogs.cpp b/Framework/DataHandling/src/LoadNexusLogs.cpp
index b41fe714454..879ca7e3a8a 100644
--- a/Framework/DataHandling/src/LoadNexusLogs.cpp
+++ b/Framework/DataHandling/src/LoadNexusLogs.cpp
@@ -20,6 +20,8 @@
#include "MantidDataHandling/LoadTOFRawNexus.h"
#include <boost/scoped_array.hpp>
+#include <algorithm>
+
namespace Mantid {
namespace DataHandling {
// Register the algorithm into the algorithm factory
@@ -128,6 +130,58 @@ bool isControlValue(const char &c, const std::string &propName,
return std::iscntrl(c, locale);
}
}
+
+/**
+ * Appends an additional entry to a TimeSeriesProperty which is at the end
+ * time of the run and contains the last value of the property recorded before
+ * the end time.
+ *
+ * This is a workaround to ensure that time series averaging of log values works
+ * correctly for instruments who do not record log values for the entire run.
+ *
+ * If the run does not have an end time or if the last time of the time series
+ * log is the same as the end time the property is left unmodified.
+ *
+ * @param prop :: a pointer to a TimeSeriesProperty to modify
+ * @param run :: handle to the run object containing the end time.
+ */
+void appendEndTimeLog(Kernel::Property *prop, const API::Run &run) {
+ try {
+ auto tsLog = dynamic_cast<TimeSeriesProperty<double> *>(prop);
+ const auto endTime = run.endTime();
+
+ // First check if it is valid to add a additional log entry
+ if (!tsLog || tsLog->size() == 0 || endTime <= tsLog->lastTime())
+ return;
+
+ tsLog->addValue(endTime, tsLog->lastValue());
+ } catch (Exception::NotFoundError) {
+ // pass
+ } catch (std::runtime_error) {
+ // pass
+ }
+}
+
+/**
+ * Read the start & end time of the run from the nexus file if they exist.
+ *
+ * @param file :: handle to the nexus file to read from.
+ * @param run :: handle to the run object to set the start & end time for.
+ */
+void readStartAndEndTime(::NeXus::File &file, API::Run &run) {
+ try {
+ // Read the start and end time strings
+ file.openData("start_time");
+ Types::Core::DateAndTime start(file.getStrData());
+ file.closeData();
+ file.openData("end_time");
+ Types::Core::DateAndTime end(file.getStrData());
+ file.closeData();
+ run.setStartAndEndTime(start, end);
+ } catch (::NeXus::Exception &) {
+ }
+}
+
} // End of anonymous namespace
/// Empty default constructor
@@ -216,6 +270,9 @@ void LoadNexusLogs::exec() {
} catch (::NeXus::Exception &) {
// No time. This is not an SNS group
}
+
+ readStartAndEndTime(file, workspace->mutableRun());
+
// print out the entry level fields
std::map<std::string, std::string> entries = file.getEntries();
std::map<std::string, std::string>::const_iterator iend = entries.end();
@@ -304,17 +361,6 @@ void LoadNexusLogs::exec() {
workspace->mutableRun().addProperty(pcharge, true);
}
}
- try {
- // Read the start and end time strings
- file.openData("start_time");
- Types::Core::DateAndTime start(file.getStrData());
- file.closeData();
- file.openData("end_time");
- Types::Core::DateAndTime end(file.getStrData());
- file.closeData();
- workspace->mutableRun().setStartAndEndTime(start, end);
- } catch (::NeXus::Exception &) {
- }
if (!workspace->run().hasProperty("gd_prtn_chrg")) {
// Try pulling it from the main proton_charge entry first
@@ -499,6 +545,7 @@ void LoadNexusLogs::loadNXLog(
try {
if (overwritelogs || !(workspace->run().hasProperty(entry_name))) {
Kernel::Property *logValue = createTimeSeries(file, entry_name);
+ appendEndTimeLog(logValue, workspace->run());
workspace->mutableRun().addProperty(logValue, overwritelogs);
}
} catch (::NeXus::Exception &e) {
@@ -539,7 +586,10 @@ void LoadNexusLogs::loadSELog(
file.closeGroup();
throw;
}
+
logValue = createTimeSeries(file, propName);
+ appendEndTimeLog(logValue, workspace->run());
+
file.closeGroup();
} catch (std::exception &e) {
g_log.warning() << "IXseblock entry '" << entry_name
diff --git a/Framework/DataHandling/test/LoadISISNexusTest.h b/Framework/DataHandling/test/LoadISISNexusTest.h
index 1306b867c25..53d8000081e 100644
--- a/Framework/DataHandling/test/LoadISISNexusTest.h
+++ b/Framework/DataHandling/test/LoadISISNexusTest.h
@@ -244,7 +244,7 @@ public:
dynamic_cast<TimeSeriesProperty<double> *>(
ws->run().getLogData("proton_charge"));
TS_ASSERT(dlog);
- TS_ASSERT_EQUALS(dlog->size(), 172);
+ TS_ASSERT_EQUALS(dlog->size(), 173);
TimeSeriesProperty<bool> *blog = dynamic_cast<TimeSeriesProperty<bool> *>(
ws->run().getLogData("period 1"));
@@ -648,7 +648,7 @@ public:
dynamic_cast<TimeSeriesProperty<double> *>(
ws->run().getLogData("proton_charge"));
TS_ASSERT(dlog);
- TS_ASSERT_EQUALS(dlog->size(), 172);
+ TS_ASSERT_EQUALS(dlog->size(), 173);
TimeSeriesProperty<bool> *blog = dynamic_cast<TimeSeriesProperty<bool> *>(
ws->run().getLogData("period 1"));
diff --git a/Framework/DataHandling/test/LoadNexusLogsTest.h b/Framework/DataHandling/test/LoadNexusLogsTest.h
index 0fe697a9e61..36f104abe7e 100644
--- a/Framework/DataHandling/test/LoadNexusLogsTest.h
+++ b/Framework/DataHandling/test/LoadNexusLogsTest.h
@@ -120,7 +120,7 @@ public:
dynamic_cast<TimeSeriesProperty<double> *>(
run.getLogData("proton_charge"));
TS_ASSERT(dlog);
- TS_ASSERT_EQUALS(dlog->size(), 172);
+ TS_ASSERT_EQUALS(dlog->size(), 173);
}
void test_File_With_Bad_Property() {
@@ -271,6 +271,29 @@ public:
TS_ASSERT_THROWS_NOTHING(loader.execute())
}
+ void test_last_time_series_log_entry_equals_end_time() {
+ LoadNexusLogs ld;
+ std::string outws_name = "REF_L_instrument";
+ ld.initialize();
+ ld.setPropertyValue("Filename", "REF_L_32035.nxs");
+ MatrixWorkspace_sptr ws = createTestWorkspace();
+ // Put it in the object.
+ ld.setProperty("Workspace", ws);
+ ld.execute();
+ TS_ASSERT(ld.isExecuted());
+
+ auto run = ws->run();
+ auto pclog = dynamic_cast<TimeSeriesProperty<double> *>(
+ run.getLogData("PhaseRequest1"));
+
+ TS_ASSERT(pclog);
+
+ const auto lastTime = pclog->lastTime();
+ const auto endTime = run.endTime();
+
+ TS_ASSERT_EQUALS(endTime.totalNanoseconds(), lastTime.totalNanoseconds());
+ }
+
private:
API::MatrixWorkspace_sptr createTestWorkspace() {
return WorkspaceFactory::Instance().create("Workspace2D", 1, 1, 1);
diff --git a/docs/source/release/v3.14.0/framework.rst b/docs/source/release/v3.14.0/framework.rst
index 13843146508..ecbfc20d335 100644
--- a/docs/source/release/v3.14.0/framework.rst
+++ b/docs/source/release/v3.14.0/framework.rst
@@ -79,6 +79,7 @@ Bugfixes
- Fixed a rare bug in :ref:`MaskDetectors <algm-MaskDetectors>` where a workspace could become invalidaded in Python if it was a ``MaskWorkspace``.
- Fixed a crash in :ref:`MaskDetectors <algm-MaskDetectors>` when a non-existent component was given in ``ComponentList``.
- History for algorithms that took groups sometimes would get incorrect history causing history to be incomplete, so now full group history is saved for all items belonging to the group.
+- Fixed a bug in `SetGoniometer <algm-SetGoniometer>` where it would use the mean log value rather than the time series average value for goniometer angles.
Python
diff --git a/qt/python/mantidqt/widgets/samplelogs/test/test_samplelogs_model.py b/qt/python/mantidqt/widgets/samplelogs/test/test_samplelogs_model.py
index 52a8e707d77..1b78f09131c 100644
--- a/qt/python/mantidqt/widgets/samplelogs/test/test_samplelogs_model.py
+++ b/qt/python/mantidqt/widgets/samplelogs/test/test_samplelogs_model.py
@@ -55,7 +55,7 @@ class SampleLogsModelTest(unittest.TestCase):
self.assertEqual(itemModel.rowCount(), 48)
self.assertEqual(itemModel.item(0,0).text(), "ChopperStatus1")
self.assertEqual(itemModel.item(0,1).text(), "float series")
- self.assertEqual(itemModel.item(0,2).text(), "4.0")
+ self.assertEqual(itemModel.item(0,2).text(), "(2 entries)")
self.assertEqual(itemModel.item(0,3).text(), "")
def test_model_MD(self):
--
GitLab