Skip to content

Commit f54a2c3

Browse files
authored
[ML] Correct query times for model plot and forecast (#327)
1 parent 1978295 commit f54a2c3

File tree

6 files changed

+60
-28
lines changed

6 files changed

+60
-28
lines changed

docs/CHANGELOG.asciidoc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@
2828
2929
//=== Regressions
3030
31+
== {es} version 6.5.3
32+
33+
=== Bug Fixes
34+
35+
Correct query times for model plot and forecast in the bucket to match the times we assign
36+
the samples we add to the model for each bucket. For long bucket lengths, this could result
37+
in apparently shifted model plot with respect to the data and increased errors in forecasts.
38+
3139
== {es} version 6.5.0
3240
3341
//=== Breaking Changes

include/api/CForecastRunner.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class API_EXPORT CForecastRunner final : private core::CNonCopyable {
112112
using TAnomalyDetectorPtr = std::shared_ptr<model::CAnomalyDetector>;
113113
using TAnomalyDetectorPtrVec = std::vector<TAnomalyDetectorPtr>;
114114

115-
using TForecastModelWrapper = model::CForecastDataSink::SForecastModelWrapper;
115+
using TForecastModelWrapper = model::CForecastDataSink::CForecastModelWrapper;
116116
using TForecastResultSeries = model::CForecastDataSink::SForecastResultSeries;
117117
using TForecastResultSeriesVec = std::vector<TForecastResultSeries>;
118118
using TMathsModelPtr = std::unique_ptr<maths::CModel>;

include/model/CForecastDataSink.h

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,31 @@ class MODEL_EXPORT CForecastDataSink final : private core::CNonCopyable {
4141
public:
4242
using TMathsModelPtr = std::shared_ptr<maths::CModel>;
4343
using TStrUMap = boost::unordered_set<std::string>;
44+
struct SForecastResultSeries;
4445

4546
//! Wrapper for 1 timeseries model, its feature and by Field
46-
struct MODEL_EXPORT SForecastModelWrapper {
47-
SForecastModelWrapper(model_t::EFeature feature,
47+
class MODEL_EXPORT CForecastModelWrapper {
48+
public:
49+
CForecastModelWrapper(model_t::EFeature feature,
4850
TMathsModelPtr&& forecastModel,
4951
const std::string& byFieldValue);
5052

51-
SForecastModelWrapper(SForecastModelWrapper&& other);
53+
CForecastModelWrapper(CForecastModelWrapper&& other);
5254

53-
SForecastModelWrapper(const SForecastModelWrapper& that) = delete;
54-
SForecastModelWrapper& operator=(const SForecastModelWrapper&) = delete;
55+
CForecastModelWrapper(const CForecastModelWrapper& that) = delete;
56+
CForecastModelWrapper& operator=(const CForecastModelWrapper&) = delete;
5557

56-
model_t::EFeature s_Feature;
57-
TMathsModelPtr s_ForecastModel;
58-
std::string s_ByFieldValue;
58+
bool forecast(const SForecastResultSeries& series,
59+
core_t::TTime startTime,
60+
core_t::TTime endTime,
61+
double boundsPercentile,
62+
CForecastDataSink& sink,
63+
std::string& message) const;
64+
65+
private:
66+
model_t::EFeature m_Feature;
67+
TMathsModelPtr m_ForecastModel;
68+
std::string m_ByFieldValue;
5969
};
6070

6171
//! Everything that defines 1 series of forecasts
@@ -69,7 +79,7 @@ class MODEL_EXPORT CForecastDataSink final : private core::CNonCopyable {
6979

7080
SModelParams s_ModelParams;
7181
int s_DetectorIndex;
72-
std::vector<SForecastModelWrapper> s_ToForecast;
82+
std::vector<CForecastModelWrapper> s_ToForecast;
7383
std::string s_ToForecastPersisted;
7484
std::string s_PartitionFieldName;
7585
std::string s_PartitionFieldValue;

lib/api/CForecastRunner.cc

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,17 +172,10 @@ void CForecastRunner::forecastWorker() {
172172
}
173173
}
174174

175-
const TForecastModelWrapper& model = series.s_ToForecast.back();
176-
model_t::TDouble1VecDouble1VecPr support =
177-
model_t::support(model.s_Feature);
178-
bool success = model.s_ForecastModel->forecast(
179-
forecastJob.s_StartTime, forecastJob.forecastEnd(),
180-
forecastJob.s_BoundsPercentile, support.first, support.second,
181-
boost::bind(&model::CForecastDataSink::push, &sink, _1,
182-
model_t::print(model.s_Feature), series.s_PartitionFieldName,
183-
series.s_PartitionFieldValue, series.s_ByFieldName,
184-
model.s_ByFieldValue, series.s_DetectorIndex),
185-
message);
175+
const TForecastModelWrapper& model{series.s_ToForecast.back()};
176+
bool success{model.forecast(
177+
series, forecastJob.s_StartTime, forecastJob.forecastEnd(),
178+
forecastJob.s_BoundsPercentile, sink, message)};
186179
series.s_ToForecast.pop_back();
187180

188181
if (success == false) {

lib/model/CForecastDataSink.cc

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include <core/CLogger.h>
1010
#include <core/CScopedRapidJsonPoolAllocator.h>
1111

12+
#include <boost/bind.hpp>
13+
1214
#include <vector>
1315

1416
namespace ml {
@@ -55,16 +57,34 @@ const std::string CForecastDataSink::STATUS("forecast_status");
5557

5658
using TScopedAllocator = core::CScopedRapidJsonPoolAllocator<core::CRapidJsonConcurrentLineWriter>;
5759

58-
CForecastDataSink::SForecastModelWrapper::SForecastModelWrapper(model_t::EFeature feature,
60+
CForecastDataSink::CForecastModelWrapper::CForecastModelWrapper(model_t::EFeature feature,
5961
TMathsModelPtr&& forecastModel,
6062
const std::string& byFieldValue)
61-
: s_Feature(feature), s_ForecastModel(std::move(forecastModel)),
62-
s_ByFieldValue(byFieldValue) {
63+
: m_Feature(feature), m_ForecastModel(std::move(forecastModel)),
64+
m_ByFieldValue(byFieldValue) {
65+
}
66+
67+
CForecastDataSink::CForecastModelWrapper::CForecastModelWrapper(CForecastModelWrapper&& other)
68+
: m_Feature(other.m_Feature), m_ForecastModel(std::move(other.m_ForecastModel)),
69+
m_ByFieldValue(std::move(other.m_ByFieldValue)) {
6370
}
6471

65-
CForecastDataSink::SForecastModelWrapper::SForecastModelWrapper(SForecastModelWrapper&& other)
66-
: s_Feature(other.s_Feature), s_ForecastModel(std::move(other.s_ForecastModel)),
67-
s_ByFieldValue(std::move(other.s_ByFieldValue)) {
72+
bool CForecastDataSink::CForecastModelWrapper::forecast(const SForecastResultSeries& series,
73+
core_t::TTime startTime,
74+
core_t::TTime endTime,
75+
double boundsPercentile,
76+
CForecastDataSink& sink,
77+
std::string& message) const {
78+
core_t::TTime bucketLength{m_ForecastModel->params().bucketLength()};
79+
startTime = model_t::sampleTime(m_Feature, startTime, bucketLength);
80+
endTime = model_t::sampleTime(m_Feature, endTime, bucketLength);
81+
model_t::TDouble1VecDouble1VecPr support{model_t::support(m_Feature)};
82+
return m_ForecastModel->forecast(
83+
startTime, endTime, boundsPercentile, support.first, support.second,
84+
boost::bind(&model::CForecastDataSink::push, &sink, _1, model_t::print(m_Feature),
85+
series.s_PartitionFieldName, series.s_PartitionFieldValue,
86+
series.s_ByFieldName, m_ByFieldValue, series.s_DetectorIndex),
87+
message);
6888
}
6989

7090
CForecastDataSink::SForecastResultSeries::SForecastResultSeries(const SModelParams& modelParams)

lib/model/CModelDetailsView.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,12 @@ void CModelDetailsView::modelPlotForByFieldId(core_t::TTime time,
7777

7878
if (this->isByFieldIdActive(byFieldId)) {
7979
const maths::CModel* model = this->model(feature, byFieldId);
80-
if (!model) {
80+
if (model == nullptr) {
8181
return;
8282
}
8383

8484
std::size_t dimension = model_t::dimension(feature);
85+
time = model_t::sampleTime(feature, time, model->params().bucketLength());
8586

8687
maths_t::TDouble2VecWeightsAry weights(
8788
maths_t::CUnitWeights::unit<TDouble2Vec>(dimension));

0 commit comments

Comments
 (0)