From e96bdef4bb29fa27be2d88905dca1f1dd75db3f2 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Thu, 26 Jan 2023 12:49:48 -0500 Subject: [PATCH 01/16] Change TimeROI to be an optional pointer --- .../inc/MantidKernel/ITimeSeriesProperty.h | 4 +- .../inc/MantidKernel/TimeSeriesProperty.h | 2 +- Framework/Kernel/src/TimeSeriesProperty.cpp | 57 ++++++++++--------- .../Kernel/test/TimeSeriesPropertyTest.h | 2 +- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Framework/Kernel/inc/MantidKernel/ITimeSeriesProperty.h b/Framework/Kernel/inc/MantidKernel/ITimeSeriesProperty.h index ce83b58ff4be..3520c6fb6d8f 100644 --- a/Framework/Kernel/inc/MantidKernel/ITimeSeriesProperty.h +++ b/Framework/Kernel/inc/MantidKernel/ITimeSeriesProperty.h @@ -44,13 +44,11 @@ class ITimeSeriesProperty { virtual std::pair averageAndStdDevInFilter(const std::vector &filter) const = 0; /// Return the time series's times as a vector virtual std::vector timesAsVector() const = 0; - /// Returns the calculated time weighted average value - virtual double timeAverageValue() const = 0; /** Returns the calculated time weighted average value. * @param timeRoi Object that holds information about when the time measurement was active. * @return The time-weighted average value of the log when the time measurement was active. */ - virtual double timeAverageValue(const TimeROI &timeRoi) const = 0; + virtual double timeAverageValue(const TimeROI *timeRoi = nullptr) const = 0; /// Return a TimeSeriesPropertyStatistics object virtual TimeSeriesPropertyStatistics getStatistics(const TimeROI *roi = nullptr) const = 0; /// Returns the real size of the time series property map: diff --git a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h index 9a053d602f42..7812cafeaeec 100644 --- a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h +++ b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h @@ -207,7 +207,7 @@ template class DLLExport TimeSeriesProperty : public Property, p * @param timeRoi Object that holds information about when the time measurement was active. * @return The time-weighted average value of the log when the time measurement was active. */ - double timeAverageValue(const TimeROI &timeRoi) const override; + double timeAverageValue(const TimeROI *timeRoi = nullptr) const override; /// generate constant time-step histogram from the property values void histogramData(const Types::Core::DateAndTime &tMin, const Types::Core::DateAndTime &tMax, std::vector &counts) const; diff --git a/Framework/Kernel/src/TimeSeriesProperty.cpp b/Framework/Kernel/src/TimeSeriesProperty.cpp index a15743c4f65f..f34dad1623dd 100644 --- a/Framework/Kernel/src/TimeSeriesProperty.cpp +++ b/Framework/Kernel/src/TimeSeriesProperty.cpp @@ -788,30 +788,20 @@ void TimeSeriesProperty::expandFilterToRange(std::vector double TimeSeriesProperty::timeAverageValue() const { - double retVal = 0.0; - try { - const auto &filter = getSplittingIntervals(); - retVal = this->averageValueInFilter(filter); - } catch (std::exception &) { - // just return nan - retVal = std::numeric_limits::quiet_NaN(); - } - return retVal; -} - /** Returns the calculated time weighted average value. * @param timeRoi Object that holds information about when the time measurement was active. * @return The time-weighted average value of the log when the time measurement was active. */ -template double TimeSeriesProperty::timeAverageValue(const TimeROI &timeRoi) const { +template double TimeSeriesProperty::timeAverageValue(const TimeROI *timeRoi) const { double retVal = 0.0; try { - const auto &filter = timeRoi.toSplitters(); - retVal = this->averageValueInFilter(filter); + if ((timeRoi == nullptr) || (timeRoi->empty())) { + const auto &filter = getSplittingIntervals(); + retVal = this->averageValueInFilter(filter); + } else { + const auto &filter = timeRoi->toSplitters(); + retVal = this->averageValueInFilter(filter); + } } catch (std::exception &) { // just return nan retVal = std::numeric_limits::quiet_NaN(); @@ -829,15 +819,18 @@ template double TimeSeriesProperty::averageValueInFilter(const std::vector &filter) const { // TODO: Consider logs that aren't giving starting values. - // First of all, if the log or the filter is empty, return NaN - if (realSize() == 0 || filter.empty()) { - return std::numeric_limits::quiet_NaN(); - } - // If there's just a single value in the log, return that. if (realSize() == 1) { return static_cast(m_values.front().value()); } + if (size() == 1) { + return static_cast(this->firstValue()); + } + + // First of all, if the log or the filter is empty, return NaN + if (realSize() == 0 || filter.empty()) { + return std::numeric_limits::quiet_NaN(); + } sortIfNecessary(); @@ -863,8 +856,14 @@ double TimeSeriesProperty::averageValueInFilter(const std::vector 0) { + // 'Normalise' by the total time + return numerator / totalTime; + } else { + // give simple mean + const auto stats = Mantid::Kernel::getStatistics(this->valuesAsVector(), Mantid::Kernel::Math::StatisticType::Mean); + return stats.mean; + } } /** Function specialization for TimeSeriesProperty @@ -2391,7 +2390,13 @@ template std::vector TimeSeriesProperty std::vector intervals; // Case where there is no filter if (m_filter.empty()) { - intervals.emplace_back(firstTime(), lastTime()); + // interval calculates what a reasonable place to put the end point for the last log entry is + // this *should* be a reasonable estimate and *is* better than always, effectively, ignoring + // the last log value. The value is different than that from lastTime() + auto lastInterval = this->nthInterval(this->size() - 1); + + intervals.emplace_back(firstTime(), lastInterval.end()); + return intervals; } diff --git a/Framework/Kernel/test/TimeSeriesPropertyTest.h b/Framework/Kernel/test/TimeSeriesPropertyTest.h index c2dd0d0a0bd6..3c19ab2e6b1b 100644 --- a/Framework/Kernel/test/TimeSeriesPropertyTest.h +++ b/Framework/Kernel/test/TimeSeriesPropertyTest.h @@ -689,7 +689,7 @@ class TimeSeriesPropertyTest : public CxxTest::TestSuite { void test_timeAverageValueWithROI() { auto dblLog = createDoubleTSP(); TimeROI *rois = createTimeRoi(); - const double dblMean = dblLog->timeAverageValue(*rois); + const double dblMean = dblLog->timeAverageValue(rois); delete dblLog; // clean up delete rois; const double expected = (5.0 * 9.99 + 5.0 * 7.55 + 5.0 * 5.55 + 5.0 * 10.55) / (5.0 + 5.0 + 5.0 + 5.0); From 35cc6e1d3493868249b77d62790945ec9ba63a3f Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Thu, 26 Jan 2023 12:55:20 -0500 Subject: [PATCH 02/16] Use TimeSeriesProperty to find mean value --- Framework/Kernel/inc/MantidKernel/LogParser.h | 3 +- Framework/Kernel/src/LogParser.cpp | 28 ++----------------- 2 files changed, 5 insertions(+), 26 deletions(-) diff --git a/Framework/Kernel/inc/MantidKernel/LogParser.h b/Framework/Kernel/inc/MantidKernel/LogParser.h index 31611b3e26e2..0ca4a671e275 100644 --- a/Framework/Kernel/inc/MantidKernel/LogParser.h +++ b/Framework/Kernel/inc/MantidKernel/LogParser.h @@ -29,6 +29,7 @@ namespace Kernel { //------------------------------------------------------------------------- // Forward declarations //------------------------------------------------------------------------- +class TimeROI; class Property; template class TimeSeriesProperty; @@ -110,7 +111,7 @@ class MANTID_KERNEL_DLL LogParser { }; /// Returns the mean value if the property is TimeSeriesProperty -MANTID_KERNEL_DLL double timeMean(const Kernel::Property *p); +MANTID_KERNEL_DLL double timeMean(const Kernel::Property *p, const TimeROI *roi = nullptr); } // namespace Kernel } // namespace Mantid diff --git a/Framework/Kernel/src/LogParser.cpp b/Framework/Kernel/src/LogParser.cpp index a229254546e2..cf41fba8c889 100644 --- a/Framework/Kernel/src/LogParser.cpp +++ b/Framework/Kernel/src/LogParser.cpp @@ -11,6 +11,7 @@ #include "MantidKernel/Logger.h" #include "MantidKernel/PropertyWithValue.h" #include "MantidKernel/Strings.h" +#include "MantidKernel/TimeROI.h" #include "MantidKernel/TimeSeriesProperty.h" #include @@ -309,37 +310,14 @@ bool LogParser::isICPEventLogNewStyle(const std::multimap */ -double timeMean(const Kernel::Property *p) { +double timeMean(const Kernel::Property *p, const Kernel::TimeROI *roi) { const auto *dp = dynamic_cast *>(p); if (!dp) { throw std::runtime_error("Property of a wrong type. Cannot be cast to a " "TimeSeriesProperty."); } - // Special case for only one value - the algorithm - if (dp->size() == 1) { - return dp->nthValue(1); - } - double res = 0.; - Types::Core::time_duration total(0, 0, 0, 0); - size_t dp_size = dp->size(); - for (size_t i = 0; i < dp_size; i++) { - Kernel::TimeInterval t = dp->nthInterval(static_cast(i)); - Types::Core::time_duration dt = t.length(); - total += dt; - res += dp->nthValue(static_cast(i)) * Types::Core::DateAndTime::secondsFromDuration(dt); - } - - double total_seconds = Types::Core::DateAndTime::secondsFromDuration(total); - - // If all the time stamps were the same, just return the first value. - if (total_seconds == 0.0) - res = dp->nthValue(1); - - if (total_seconds > 0) - res /= total_seconds; - - return res; + return dp->timeAverageValue(roi); } } // namespace Kernel From e4945d864ace3ab8699d7cab62e7c3d61057f2ee Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Thu, 26 Jan 2023 18:31:46 -0500 Subject: [PATCH 03/16] Change the value of correct --- .../Kernel/test/TimeSeriesPropertyTest.h | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Framework/Kernel/test/TimeSeriesPropertyTest.h b/Framework/Kernel/test/TimeSeriesPropertyTest.h index 3c19ab2e6b1b..68171c3c27e7 100644 --- a/Framework/Kernel/test/TimeSeriesPropertyTest.h +++ b/Framework/Kernel/test/TimeSeriesPropertyTest.h @@ -672,14 +672,13 @@ class TimeSeriesPropertyTest : public CxxTest::TestSuite { } void test_timeAverageValue() { + // values are equally spaced auto dblLog = createDoubleTSP(); auto intLog = createIntegerTSP(5); // average values - const double dblMean = dblLog->timeAverageValue(); - TS_ASSERT_DELTA(dblMean, 7.6966, .0001); - const double intMean = intLog->timeAverageValue(); - TS_ASSERT_DELTA(intMean, 2.5, .0001); + TS_ASSERT_DELTA(dblLog->timeAverageValue(), dblLog->mean(), .0001); + TS_ASSERT_DELTA(intLog->timeAverageValue(), intLog->mean(), .0001); // Clean up delete dblLog; @@ -1096,11 +1095,11 @@ class TimeSeriesPropertyTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(stats.maximum, 11.0, 1e-3); TS_ASSERT_DELTA(stats.median, 6.0, 1e-3); TS_ASSERT_DELTA(stats.mean, 6.0, 1e-3); - TS_ASSERT_DELTA(stats.duration, 100.0, 1e-3); + TS_ASSERT_DELTA(stats.duration, 110.0, 1e-3); TS_ASSERT_DELTA(stats.standard_deviation, 3.1622, 1e-3); - TS_ASSERT_DELTA(log->timeAverageValue(), 5.5, 1e-3); - TS_ASSERT_DELTA(stats.time_mean, 5.5, 1e-3); - TS_ASSERT_DELTA(stats.time_standard_deviation, 2.872, 1e-3); + TS_ASSERT_DELTA(log->timeAverageValue(), stats.mean, 1e-3); + TS_ASSERT_DELTA(stats.time_mean, stats.mean, 1e-3); + TS_ASSERT_DELTA(stats.time_standard_deviation, stats.standard_deviation, 1e-3); delete log; } @@ -2262,7 +2261,7 @@ class TimeSeriesPropertyTest : public CxxTest::TestSuite { void test_filterByTime_out_of_range_filters_nothing() { TimeSeriesProperty *log = createIntegerTSP(6); - size_t original_size = log->realSize(); + size_t original_size = std::size_t(log->realSize()); TS_ASSERT_EQUALS(original_size, 6); @@ -2313,7 +2312,12 @@ class TimeSeriesPropertyTest : public CxxTest::TestSuite { TS_ASSERT_EQUALS(intervals.size(), 1); const auto &range = intervals.front(); TS_ASSERT_EQUALS(range.begin(), log->firstTime()); - TS_ASSERT_EQUALS(range.end(), log->lastTime()); + + // the range is extended by the last difference in times + // this is to make the last value count as much as the penultimate + const auto lastDuration = log->nthInterval(log->size() - 1).length(); + const auto stop = log->lastTime() + lastDuration; + TS_ASSERT_EQUALS(range.end(), stop); } void test_getSplittingIntervals_repeatedEntries() { From 84242e3edd99e2f6e516499feabe0f829a376888 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Thu, 26 Jan 2023 18:55:35 -0500 Subject: [PATCH 04/16] Use default value rather than overloading --- .../mantid/kernel/src/Exports/TimeSeriesProperty.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp index a5c4b0ca4feb..c0da4a383b00 100644 --- a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp +++ b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp @@ -113,12 +113,7 @@ template <> std::string dtype(TimeSeriesProperty &self) { .def("nthTime", &TimeSeriesProperty::nthTime, (arg("self"), arg("index")), \ "returns :class:`mantid.kernel.DateAndTime`") \ .def("getStatistics", &TimeSeriesProperty::getStatistics, getStatistics_overloads()) \ - .def("timeAverageValue", \ - (double (TimeSeriesProperty::*)() const) & TimeSeriesProperty::timeAverageValue, (arg("self"))) \ - .def("timeAverageValue", \ - (double (TimeSeriesProperty::*)(const Mantid::Kernel::TimeROI &) const) & \ - TimeSeriesProperty::timeAverageValue, \ - (arg("self"), arg("time_roi"))) \ + .def("timeAverageValue", &TimeSeriesProperty::timeAverageValue, (arg("self"), arg("time_roi") = nullptr)) \ .def("dtype", &dtype, arg("self")); } // namespace From c1ec2b958912a24e66c522d079c8ebe3c9936fe9 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 31 Jan 2023 11:29:20 -0500 Subject: [PATCH 05/16] Fix overloaded export --- .../mantid/kernel/src/Exports/TimeSeriesProperty.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp index c0da4a383b00..72a82779311c 100644 --- a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp +++ b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -74,6 +75,8 @@ template <> std::string dtype(TimeSeriesProperty &self) { return retVal; } +BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(timeAverageValue_Overloads, timeAverageValue, 0, 1); + // Macro to reduce copy-and-paste #define EXPORT_TIMESERIES_PROP(TYPE, Prefix) \ register_ptr_to_python *>(); \ @@ -113,7 +116,8 @@ template <> std::string dtype(TimeSeriesProperty &self) { .def("nthTime", &TimeSeriesProperty::nthTime, (arg("self"), arg("index")), \ "returns :class:`mantid.kernel.DateAndTime`") \ .def("getStatistics", &TimeSeriesProperty::getStatistics, getStatistics_overloads()) \ - .def("timeAverageValue", &TimeSeriesProperty::timeAverageValue, (arg("self"), arg("time_roi") = nullptr)) \ + .def("timeAverageValue", &TimeSeriesProperty::timeAverageValue, \ + timeAverageValue_Overloads((arg("self"), arg("time_roi")))) \ .def("dtype", &dtype, arg("self")); } // namespace From 2dd421830231f117faa146aaa05bbe6c0bf350df Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 31 Jan 2023 12:05:01 -0500 Subject: [PATCH 06/16] Remove unimplemented method --- Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h index 7812cafeaeec..c387d55e069b 100644 --- a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h +++ b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h @@ -202,7 +202,6 @@ template class DLLExport TimeSeriesProperty : public Property, p /// @copydoc Mantid::Kernel::ITimeSeriesProperty::timeAverageValue() /// Time weighted mean and standard deviation std::pair timeAverageValueAndStdDev() const; - double timeAverageValue() const override; /** Returns the calculated time weighted average value. * @param timeRoi Object that holds information about when the time measurement was active. * @return The time-weighted average value of the log when the time measurement was active. From 1889a68ca380add83384b1d707f669d4c2d198a0 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 31 Jan 2023 15:18:33 -0500 Subject: [PATCH 07/16] Fix doxygen warning --- Framework/Kernel/src/LogParser.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Framework/Kernel/src/LogParser.cpp b/Framework/Kernel/src/LogParser.cpp index cf41fba8c889..3a284f2a4792 100644 --- a/Framework/Kernel/src/LogParser.cpp +++ b/Framework/Kernel/src/LogParser.cpp @@ -307,6 +307,7 @@ bool LogParser::isICPEventLogNewStyle(const std::multimap. + * @param roi :: TimeROI for when the log is being used * @return The mean value over time. * @throw runtime_error if the property is not TimeSeriesProperty */ From 3367d6ab6fc0f2f22a256ef0c02d4b72eff20bc6 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 31 Jan 2023 15:36:18 -0500 Subject: [PATCH 08/16] Fix doxygen warning --- Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h index c387d55e069b..b4385f6f93f1 100644 --- a/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h +++ b/Framework/Kernel/inc/MantidKernel/TimeSeriesProperty.h @@ -199,8 +199,9 @@ template class DLLExport TimeSeriesProperty : public Property, p double averageValueInFilter(const std::vector &filter) const override; /// @copydoc Mantid::Kernel::ITimeSeriesProperty::averageAndStdDevInFilter() std::pair averageAndStdDevInFilter(const std::vector &filter) const override; - /// @copydoc Mantid::Kernel::ITimeSeriesProperty::timeAverageValue() - /// Time weighted mean and standard deviation + /** Returns the calculated time weighted mean and standard deviation values. + * @return The time-weighted average value of the log when the time measurement was active. + */ std::pair timeAverageValueAndStdDev() const; /** Returns the calculated time weighted average value. * @param timeRoi Object that holds information about when the time measurement was active. From 06857d5e3cf0af1569fca6a29aad5da0467a92fc Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 31 Jan 2023 17:29:57 -0500 Subject: [PATCH 09/16] Fix broken unit tests Some of the test had their values of correct changed to reflect new contribution of final time interval based on bugfix. --- Framework/API/test/LogManagerTest.h | 30 ++++++++++----------- Framework/Kernel/src/TimeSeriesProperty.cpp | 17 +++++++++--- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Framework/API/test/LogManagerTest.h b/Framework/API/test/LogManagerTest.h index c844681faa7c..1295eec9552e 100644 --- a/Framework/API/test/LogManagerTest.h +++ b/Framework/API/test/LogManagerTest.h @@ -359,7 +359,7 @@ class LogManagerTest : public CxxTest::TestSuite { // all values were calculated using a independent implementation in python const double FIRST_VALUE{2.}; // also the min const double LAST_VALUE{24.}; // also the max - const double TIME_AVG_MEAN{15.357142857142858}; + const double TIME_AVG_MEAN{18.2380952348}; // const double TIME_AVG_STDDEV {8.523975789812294}; TS_ASSERT_EQUALS(runInfo.getProperty(name)->size(), 10); @@ -371,7 +371,7 @@ class LogManagerTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::LastValue), LAST_VALUE, 1e-12); TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::Median), 13.0, 1e-12); TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::StdDev), 9.1104335791443, 1e-12); - TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAveragedMean), TIME_AVG_MEAN, 1e-12); + TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAveragedMean), TIME_AVG_MEAN, 1e-8); // TODO not ready // TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAverageStdDev), TIME_AVG_STDDEV, 1e-12); @@ -389,7 +389,7 @@ class LogManagerTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::LastValue), LAST_VALUE, 1e-12); // TODO TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::Median), 6.0, 1e-12); // TODO TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::StdDev), 8.937367800973425, 1e-12); // TODO - TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAveragedMean), TIME_AVG_MEAN, 1e-12); + TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAveragedMean), TIME_AVG_MEAN, 1e-8); // TODO not ready // TS_ASSERT_DELTA(runInfo.getPropertyAsSingleValue(name, Math::TimeAverageStdDev), TIME_AVG_STDDEV, 1e-12); } @@ -425,7 +425,7 @@ class LogManagerTest : public CxxTest::TestSuite { const std::string name = "series"; addTestTimeSeries(run, name); - TS_ASSERT_DELTA(run.getTimeAveragedStd(name), 8.5239, 0.001); + TS_ASSERT_DELTA(run.getTimeAveragedStd(name), 8.0646, 0.001); } void test_getStatistics() { @@ -441,7 +441,7 @@ class LogManagerTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(stats.standard_deviation, 0.0, 0.001); TS_ASSERT_DELTA(stats.time_mean, value, 0.001); TS_ASSERT_DELTA(stats.time_standard_deviation, 0.0, 0.001); - TS_ASSERT(isnan(stats.duration)); + TS_ASSERT(std::isnan(stats.duration)); }; // test valid single value property @@ -458,13 +458,13 @@ class LogManagerTest : public CxxTest::TestSuite { { addTestPropertyWithValue(run, "single-string", "46"); auto stats = run.getStatistics("single-string"); - TS_ASSERT_EQUALS(isnan(stats.minimum), true); - TS_ASSERT_EQUALS(isnan(stats.maximum), true); - TS_ASSERT_EQUALS(isnan(stats.mean), true); - TS_ASSERT_EQUALS(isnan(stats.standard_deviation), true); - TS_ASSERT_EQUALS(isnan(stats.time_mean), true); - TS_ASSERT_EQUALS(isnan(stats.time_standard_deviation), true); - TS_ASSERT_EQUALS(isnan(stats.duration), true); + TS_ASSERT_EQUALS(std::isnan(stats.minimum), true); + TS_ASSERT_EQUALS(std::isnan(stats.maximum), true); + TS_ASSERT_EQUALS(std::isnan(stats.mean), true); + TS_ASSERT_EQUALS(std::isnan(stats.standard_deviation), true); + TS_ASSERT_EQUALS(std::isnan(stats.time_mean), true); + TS_ASSERT_EQUALS(std::isnan(stats.time_standard_deviation), true); + TS_ASSERT_EQUALS(std::isnan(stats.duration), true); } // test time series @@ -480,9 +480,9 @@ class LogManagerTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(stats.mean, 13.0, 0.001); TS_ASSERT_DELTA(stats.median, 13.0, 0.001); TS_ASSERT_DELTA(stats.standard_deviation, 9.1104, 0.001); - TS_ASSERT_DELTA(stats.time_mean, 15.3571, 0.001); - TS_ASSERT_DELTA(stats.time_standard_deviation, 8.5239, 0.001); - TS_ASSERT_DELTA(stats.duration, 140.0, 0.001); + TS_ASSERT_DELTA(stats.time_mean, 18.2381, 0.001); + TS_ASSERT_DELTA(stats.time_standard_deviation, 8.06464, 0.001); + TS_ASSERT_DELTA(stats.duration, 210.0, 0.001); } } diff --git a/Framework/Kernel/src/TimeSeriesProperty.cpp b/Framework/Kernel/src/TimeSeriesProperty.cpp index f34dad1623dd..96af868f8dd8 100644 --- a/Framework/Kernel/src/TimeSeriesProperty.cpp +++ b/Framework/Kernel/src/TimeSeriesProperty.cpp @@ -1550,10 +1550,19 @@ template TimeInterval TimeSeriesProperty::nthInterval(int ; } else if (n == static_cast(m_values.size()) - 1) { // 2. Last one by making up an end time. - time_duration d = m_values.rbegin()->time() - (m_values.rbegin() + 1)->time(); - DateAndTime endTime = m_values.rbegin()->time() + d; - Kernel::TimeInterval dt(m_values.rbegin()->time(), endTime); - deltaT = dt; + // the last time is the last thing known + const auto ultimate = m_values.rbegin()->time(); + // go backwards from the time before it that is different + int counter = 0; + while (DateAndTime::secondsFromDuration(ultimate - (m_values.rbegin() + counter)->time()) == 0.) { + counter += 1; + } + // get the last time that is different + time_duration lastDuration = m_values.rbegin()->time() - (m_values.rbegin() + counter)->time(); + // the last duration is equal to the previous, non-zero, duration + DateAndTime endTime = m_values.rbegin()->time() + lastDuration; + + deltaT = Kernel::TimeInterval(m_values.rbegin()->time(), endTime); } else { // 3. Regular DateAndTime startT = m_values[static_cast(n)].time(); From 80c91717f7d81b56728c6458d2b7e30eac9ea9b6 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Wed, 1 Feb 2023 09:14:19 -0500 Subject: [PATCH 10/16] Update test now that mean and time avg are equal --- .../plugins/algorithms/ExportSampleLogsToHDF5Test.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/ExportSampleLogsToHDF5Test.py b/Framework/PythonInterface/test/python/plugins/algorithms/ExportSampleLogsToHDF5Test.py index 95431dd1c94a..7fed5d96cb75 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/ExportSampleLogsToHDF5Test.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/ExportSampleLogsToHDF5Test.py @@ -60,12 +60,15 @@ def test_blacklistExcludesLogs(self): def test_timeSeriesAreTimeAveraged(self): input_ws = self._create_sample_workspace() - self._add_log_to_workspace(input_ws, "TestLog", [1.0, 2.0, 3.0]) + VALUES = [1.0, 2.0, 3.0] + self._add_log_to_workspace(input_ws, "TestLog", VALUES) run_algorithm(self.ALG_NAME, InputWorkspace=input_ws, Filename=self.TEMP_FILE_NAME) + # time average mean is equal to simple mean + # because the values are equally spaced with h5py.File(self.TEMP_FILE_NAME, "r") as output_file: logs_group = output_file["Sample Logs"] - self.assertEqual(logs_group["TestLog"][()], 1.5) + self.assertEqual(logs_group["TestLog"][()], np.mean(VALUES)) def test_unitAreAddedIfPresent(self): input_ws = self._create_sample_workspace() From 3f4aae0036840211b16d0581b296dc96e428316e Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Wed, 1 Feb 2023 09:17:07 -0500 Subject: [PATCH 11/16] Use all values in comparison --- Framework/PythonInterface/test/python/mantid/api/RunTest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Framework/PythonInterface/test/python/mantid/api/RunTest.py b/Framework/PythonInterface/test/python/mantid/api/RunTest.py index cc92d90fe22e..bcd1b6282e26 100644 --- a/Framework/PythonInterface/test/python/mantid/api/RunTest.py +++ b/Framework/PythonInterface/test/python/mantid/api/RunTest.py @@ -130,7 +130,7 @@ def test_timestd(self): temp1.addValue(start_time + i * nanosec, vals[i]) run.addProperty(temp1.name, temp1, True) # ignore the last value - expected = vals[:-1].std() + expected = vals.std() self.assertEqual(run.getTimeAveragedStd("TEMP1"), expected) def do_test_copyable(self, copy_op): From 8413d672447890fba46fc60084d2c33a3116e86e Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Wed, 1 Feb 2023 15:22:23 -0500 Subject: [PATCH 12/16] Update the expected values for the tests --- Framework/MDAlgorithms/test/ConvertToMDMinMaxLocalTest.h | 6 +++--- .../mantidqtinterfaces/test/Muon/fit_information_test.py | 6 +++--- .../test/Muon/results_tab_widget/results_tab_model_test.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Framework/MDAlgorithms/test/ConvertToMDMinMaxLocalTest.h b/Framework/MDAlgorithms/test/ConvertToMDMinMaxLocalTest.h index 7f1b6c8f1f78..c49f83e56541 100644 --- a/Framework/MDAlgorithms/test/ConvertToMDMinMaxLocalTest.h +++ b/Framework/MDAlgorithms/test/ConvertToMDMinMaxLocalTest.h @@ -184,8 +184,8 @@ class ConvertToMDMinMaxLocalTest : public CxxTest::TestSuite { TS_ASSERT_THROWS_NOTHING(alg.execute();); TS_ASSERT(alg.isExecuted()); // Check the results - TS_ASSERT_EQUALS(alg.getPropertyValue("MinValues"), "0.12187,7.69667"); - TS_ASSERT_EQUALS(alg.getPropertyValue("MaxValues"), "0.126745,7.69667"); + TS_ASSERT_EQUALS(alg.getPropertyValue("MinValues"), "0.12187,8.41"); + TS_ASSERT_EQUALS(alg.getPropertyValue("MaxValues"), "0.126745,8.41"); // Remove workspace from the data service. Mantid::API::AnalysisDataService::Instance().remove(WSName); } @@ -232,7 +232,7 @@ class ConvertToMDMinMaxLocalTest : public CxxTest::TestSuite { ws->mutableSample().setOrientedLattice(std::make_unique(2, 3, 4, 90, 90, 90)); // time average value of this is the simple average - // of the first three values = 7.69667 + // of the values = 8.41 Mantid::Kernel::TimeSeriesProperty *p = new Mantid::Kernel::TimeSeriesProperty("doubleProp"); TS_ASSERT_THROWS_NOTHING(p->addValue("2007-11-30T16:17:00", 9.99)); TS_ASSERT_THROWS_NOTHING(p->addValue("2007-11-30T16:17:10", 7.55)); diff --git a/qt/python/mantidqtinterfaces/test/Muon/fit_information_test.py b/qt/python/mantidqtinterfaces/test/Muon/fit_information_test.py index f91ff735c004..82ac46ce9745 100644 --- a/qt/python/mantidqtinterfaces/test/Muon/fit_information_test.py +++ b/qt/python/mantidqtinterfaces/test/Muon/fit_information_test.py @@ -168,7 +168,7 @@ def test_time_series_log_value_from_fit_with_single_workspace_uses_time_average( fake1 = create_test_workspace("fake1", time_series_logs) fit = FitInformation(mock.MagicMock(), "func1", [StaticWorkspaceWrapper(fake1.name(), fake1)], mock.MagicMock(), mock.MagicMock()) - time_average = (10 * 5 + 290 * 20) / 300.0 + time_average = (10 * 5 + 290 * 20 + 290 * 30) / 590.0 self.assertAlmostEqual(time_average, fit.log_value("ts_1"), places=6) def test_time_series_log_value_from_fit_with_multiple_workspaces_uses_average_of_time_average(self): @@ -184,8 +184,8 @@ def test_time_series_log_value_from_fit_with_multiple_workspaces_uses_average_of mock.MagicMock(), ) - time_average1 = (10 * 5 + 290 * 20) / 300.0 - time_average2 = (75 * 10 + 195 * 30) / 270.0 + time_average1 = (10 * 5 + 290 * 20 + 290 * 30) / 590 + time_average2 = (75 * 10 + 195 * 30 + 195 * 40) / 465 all_average = 0.5 * (time_average1 + time_average2) self.assertAlmostEqual(all_average, fit.log_value("ts_1"), places=6) diff --git a/qt/python/mantidqtinterfaces/test/Muon/results_tab_widget/results_tab_model_test.py b/qt/python/mantidqtinterfaces/test/Muon/results_tab_widget/results_tab_model_test.py index e1d5be362c11..83034fcc6b08 100644 --- a/qt/python/mantidqtinterfaces/test/Muon/results_tab_widget/results_tab_model_test.py +++ b/qt/python/mantidqtinterfaces/test/Muon/results_tab_widget/results_tab_model_test.py @@ -315,14 +315,14 @@ def test_create_results_table_with_logs_selected(self): TableColumnType.YErr, TableColumnType.Y, ) - avg_log_values = "1970-01-01T00:00:01 to 1970-01-01T00:00:01", 1, 86.0, 2.0 + avg_log_values = "1970-01-01T00:00:01 to 1970-01-01T00:00:01", 1, 90.057, 2.0 expected_content = [ ( "ws1_Parameters", avg_log_values[0], avg_log_values[1], avg_log_values[2], - 17.146, + 15.848, avg_log_values[3], 0.0, self.f0_height[0], From a7cef23315722fe3a9ba71b479e23c6c145b8c90 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Wed, 1 Feb 2023 15:29:28 -0500 Subject: [PATCH 13/16] Add release notes --- docs/source/release/v6.7.0/Framework/Python/Bugfixes/35093.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/source/release/v6.7.0/Framework/Python/Bugfixes/35093.rst diff --git a/docs/source/release/v6.7.0/Framework/Python/Bugfixes/35093.rst b/docs/source/release/v6.7.0/Framework/Python/Bugfixes/35093.rst new file mode 100644 index 000000000000..4f570a754f49 --- /dev/null +++ b/docs/source/release/v6.7.0/Framework/Python/Bugfixes/35093.rst @@ -0,0 +1 @@ +* Updated the value returned by ``TimeSeriesProperty`` for time average mean and standard deviation. This now accounts for the last point in a log which was previously, in v6.5.0, being ignored. From 0acb21ee578e737f9c2ec9af6df8af718334d0a4 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Thu, 2 Feb 2023 11:38:50 -0500 Subject: [PATCH 14/16] Remove redundate if case --- Framework/Kernel/src/TimeSeriesProperty.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/Framework/Kernel/src/TimeSeriesProperty.cpp b/Framework/Kernel/src/TimeSeriesProperty.cpp index 96af868f8dd8..f4b0ed0760a3 100644 --- a/Framework/Kernel/src/TimeSeriesProperty.cpp +++ b/Framework/Kernel/src/TimeSeriesProperty.cpp @@ -820,9 +820,6 @@ double TimeSeriesProperty::averageValueInFilter(const std::vector(m_values.front().value()); - } if (size() == 1) { return static_cast(this->firstValue()); } From 6e768734bb20e6072f29b9fea18223592eb103a6 Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 7 Feb 2023 10:50:13 -0500 Subject: [PATCH 15/16] Update reference files --- .../tests/framework/reference/MDNormBackgroundHYSPEC.nxs.md5 | 2 +- .../SystemTests/tests/framework/reference/MDNormHYSPEC.nxs.md5 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Testing/SystemTests/tests/framework/reference/MDNormBackgroundHYSPEC.nxs.md5 b/Testing/SystemTests/tests/framework/reference/MDNormBackgroundHYSPEC.nxs.md5 index 358877660993..6717b6bc1cf6 100644 --- a/Testing/SystemTests/tests/framework/reference/MDNormBackgroundHYSPEC.nxs.md5 +++ b/Testing/SystemTests/tests/framework/reference/MDNormBackgroundHYSPEC.nxs.md5 @@ -1 +1 @@ -ce913b7a314203cacd13879c14e4ed78 +4141f144d4f2e5853743c58b7e1cce05 diff --git a/Testing/SystemTests/tests/framework/reference/MDNormHYSPEC.nxs.md5 b/Testing/SystemTests/tests/framework/reference/MDNormHYSPEC.nxs.md5 index 11d3591f43d7..f542ca587b2b 100644 --- a/Testing/SystemTests/tests/framework/reference/MDNormHYSPEC.nxs.md5 +++ b/Testing/SystemTests/tests/framework/reference/MDNormHYSPEC.nxs.md5 @@ -1 +1 @@ -c2610ae5c15704573e63d30dfe0c2827 +78f7b855ea2bb223740a99be5fbd732c From 0a1454d3464f7da71487491763d52c1be30d533c Mon Sep 17 00:00:00 2001 From: Pete Peterson Date: Tue, 7 Feb 2023 14:01:23 -0500 Subject: [PATCH 16/16] "Fix" compiler warnings --- .../mantid/kernel/src/Exports/TimeSeriesProperty.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp index 72a82779311c..a9570173f3a4 100644 --- a/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp +++ b/Framework/PythonInterface/mantid/kernel/src/Exports/TimeSeriesProperty.cpp @@ -75,7 +75,10 @@ template <> std::string dtype(TimeSeriesProperty &self) { return retVal; } -BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(timeAverageValue_Overloads, timeAverageValue, 0, 1); +// Ignore -Wconversion warnings coming from boost::python +// Seen with GCC 7.1.1 and Boost 1.63.0 +GNU_DIAG_OFF("conversion") +BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(timeAverageValue_Overloads, timeAverageValue, 0, 1) // Macro to reduce copy-and-paste #define EXPORT_TIMESERIES_PROP(TYPE, Prefix) \ @@ -119,6 +122,7 @@ BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(timeAverageValue_Overloads, timeAverageVa .def("timeAverageValue", &TimeSeriesProperty::timeAverageValue, \ timeAverageValue_Overloads((arg("self"), arg("time_roi")))) \ .def("dtype", &dtype, arg("self")); +GNU_DIAG_ON("conversion") } // namespace