Skip to content

[ML] Fix bug upgrading state from 5.6 direct to 6.3 causing SIGSEGV on open job #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 4, 2018
Merged

[ML] Fix bug upgrading state from 5.6 direct to 6.3 causing SIGSEGV on open job #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b9121f
Fix bugs upgrading state from 5.6 direct to 6.3
tveasey Jul 4, 2018
452216d
Documentation
tveasey Jul 4, 2018
6397c90
Review comments
tveasey Jul 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Fix corner case failing to calculate lgamma values and the correspoinding log er
Function description for population lat_long results should be lat_long instead of mean ({pull}81[#81])
By-fields should respect model_plot_config.terms ({pull}86[#86])
The trend decomposition state wasn't being correctly upgraded potentially causing the autodetect process to abort ({pull}136[#136])
Fix a SIGSEGV in the autodetect process when jump upgrading from 5.6 to 6.3 ({pull}143[#143])

=== Regressions

Expand Down
1 change: 1 addition & 0 deletions include/maths/CTimeSeriesDecompositionDetail.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ class MATHS_EXPORT CTimeSeriesDecompositionDetail {

//! Initialize by reading state from \p traverser.
bool acceptRestoreTraverser(const SDistributionRestoreParams& params,
core_t::TTime lastValueTime,
core::CStateRestoreTraverser& traverser);

//! Persist state by passing information to \p inserter.
Expand Down
6 changes: 5 additions & 1 deletion lib/maths/CSeasonalComponentAdaptiveBucketing.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,11 @@ bool CSeasonalComponentAdaptiveBucketing::acceptRestoreTraverser(core::CStateRes
RESTORE(LAST_UPDATES_OLD_TAG,
core::CPersistUtils::fromString(traverser.value(), lastUpdates))
} while (traverser.next());

// This wasn't present in version 5.6 so needs to be default
// initialised if it is missing from the state object.
if (lastUpdates.empty()) {
lastUpdates.resize(regressions.size(), UNSET_TIME);
}
m_Buckets.clear();
m_Buckets.reserve(regressions.size());
for (std::size_t i = 0u; i < regressions.size(); ++i) {
Expand Down
14 changes: 8 additions & 6 deletions lib/maths/CTimeSeriesDecomposition.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,10 @@ bool CTimeSeriesDecomposition::acceptRestoreTraverser(const SDistributionRestore
RESTORE(CALENDAR_CYCLIC_TEST_6_3_TAG,
traverser.traverseSubLevel(boost::bind(&CCalendarTest::acceptRestoreTraverser,
&m_CalendarCyclicTest, _1)))
RESTORE(COMPONENTS_6_3_TAG, traverser.traverseSubLevel(boost::bind(
&CComponents::acceptRestoreTraverser,
&m_Components, boost::cref(params), _1)))
RESTORE(COMPONENTS_6_3_TAG,
traverser.traverseSubLevel(
boost::bind(&CComponents::acceptRestoreTraverser, &m_Components,
boost::cref(params), m_LastValueTime, _1)))
}
} else {
// There is no version string this is historic state.
Expand All @@ -149,9 +150,10 @@ bool CTimeSeriesDecomposition::acceptRestoreTraverser(const SDistributionRestore
RESTORE(CALENDAR_CYCLIC_TEST_OLD_TAG,
traverser.traverseSubLevel(boost::bind(&CCalendarTest::acceptRestoreTraverser,
&m_CalendarCyclicTest, _1)))
RESTORE(COMPONENTS_OLD_TAG, traverser.traverseSubLevel(boost::bind(
&CComponents::acceptRestoreTraverser,
&m_Components, boost::cref(params), _1)))
RESTORE(COMPONENTS_OLD_TAG,
traverser.traverseSubLevel(
boost::bind(&CComponents::acceptRestoreTraverser, &m_Components,
boost::cref(params), m_LastValueTime, _1)))
} while (traverser.next());
this->decayRate(decayRate);
}
Expand Down
17 changes: 9 additions & 8 deletions lib/maths/CTimeSeriesDecompositionDetail.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,23 +323,22 @@ const std::string LAST_UPDATE_OLD_TAG{"j"};

const double MODEL_WEIGHT_UPGRADING_TO_VERSION_6_3{48.0};

bool upgradeTrendModelToVersion6p3(const core_t::TTime bucketLength,
CTrendComponent& trend,
core::CStateRestoreTraverser& traverser) {
bool upgradeTrendModelToVersion_6_3(const core_t::TTime bucketLength,
const core_t::TTime lastValueTime,
CTrendComponent& trend,
core::CStateRestoreTraverser& traverser) {
using TRegression = CRegression::CLeastSquaresOnline<3, double>;

TRegression regression;
double variance{0.0};
core_t::TTime origin{0};
core_t::TTime lastUpdate{0};
do {
const std::string& name{traverser.name()};
RESTORE(REGRESSION_OLD_TAG,
traverser.traverseSubLevel(boost::bind(
&TRegression::acceptRestoreTraverser, &regression, _1)))
RESTORE_BUILT_IN(VARIANCE_OLD_TAG, variance)
RESTORE_BUILT_IN(TIME_ORIGIN_OLD_TAG, origin)
RESTORE_BUILT_IN(LAST_UPDATE_OLD_TAG, lastUpdate)
} while (traverser.next());

// Generate some samples from the old trend model.
Expand All @@ -348,7 +347,8 @@ bool upgradeTrendModelToVersion6p3(const core_t::TTime bucketLength,
static_cast<double>(bucketLength) / static_cast<double>(4 * WEEK)};

CPRNG::CXorOShiro128Plus rng;
for (core_t::TTime time = lastUpdate - 4 * WEEK; time < lastUpdate; time += bucketLength) {
for (core_t::TTime time = lastValueTime - 4 * WEEK; time < lastValueTime;
time += bucketLength) {
double time_{static_cast<double>(time - origin) / static_cast<double>(WEEK)};
double sample{regression.predict(time_) + CSampling::normalSample(rng, 0.0, variance)};
trend.add(time, sample, weight);
Expand Down Expand Up @@ -1010,6 +1010,7 @@ CTimeSeriesDecompositionDetail::CComponents::CComponents(const CComponents& othe

bool CTimeSeriesDecompositionDetail::CComponents::acceptRestoreTraverser(
const SDistributionRestoreParams& params,
core_t::TTime lastValueTime,
core::CStateRestoreTraverser& traverser) {
if (traverser.name() == VERSION_6_3_TAG) {
while (traverser.next()) {
Expand Down Expand Up @@ -1059,8 +1060,8 @@ bool CTimeSeriesDecompositionDetail::CComponents::acceptRestoreTraverser(
RESTORE_SETUP_TEARDOWN(TREND_OLD_TAG,
/**/,
traverser.traverseSubLevel(boost::bind(
upgradeTrendModelToVersion6p3,
m_BucketLength, boost::ref(m_Trend), _1)),
upgradeTrendModelToVersion_6_3, m_BucketLength,
lastValueTime, boost::ref(m_Trend), _1)),
m_UsingTrendForPrediction = true)
RESTORE_SETUP_TEARDOWN(
SEASONAL_OLD_TAG, m_Seasonal = boost::make_unique<CSeasonal>(),
Expand Down
68 changes: 68 additions & 0 deletions lib/maths/unittest/CTimeSeriesDecompositionTest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2074,6 +2074,7 @@ void CTimeSeriesDecompositionTest::testUpgrade() {
0.1, HALF_HOUR, maths::SDistributionRestoreParams{maths_t::E_ContinuousData, 0.1}};
std::string empty;

LOG_DEBUG(<< "**** From 6.2 ****");
LOG_DEBUG(<< "*** Seasonal and Calendar Components ***");
{
std::string xml;
Expand Down Expand Up @@ -2218,6 +2219,73 @@ void CTimeSeriesDecompositionTest::testUpgrade() {
decomposition.addPoint(time, 10.0);
}
}

LOG_DEBUG(<< "**** From 5.6 ****");
LOG_DEBUG(<< "*** Seasonal and Calendar Components ***");
{
std::string xml;
load("testfiles/CTimeSeriesDecomposition.5.6.seasonal.state.xml", xml);
LOG_DEBUG(<< "Saved state size = " << xml.size());

core::CRapidXmlParser parser;
CPPUNIT_ASSERT(parser.parseStringIgnoreCdata(xml));
core::CRapidXmlStateRestoreTraverser traverser(parser);

maths::CTimeSeriesDecomposition decomposition(params, traverser);

// Check that the decay rates match and the values and variances
// predictions match the values obtained from 6.2.

CPPUNIT_ASSERT_EQUAL(0.01, decomposition.decayRate());

double meanValue{decomposition.meanValue(18316800)};
double meanVariance{decomposition.meanVariance()};
LOG_DEBUG(<< "restored mean value = " << meanValue);
LOG_DEBUG(<< "restored mean variance = " << meanVariance);
CPPUNIT_ASSERT_DOUBLES_EQUAL(9.91269, meanValue, 0.005);
CPPUNIT_ASSERT_DOUBLES_EQUAL(3.99723, meanVariance, 0.5);

// Check some basic operations on the upgraded model.
decomposition.forecast(60480000, 60480000 + WEEK, HALF_HOUR, 90.0, 1.0,
[](core_t::TTime, const TDouble3Vec&) {});
for (core_t::TTime time = 60480000; time < 60480000 + WEEK; time += HALF_HOUR) {
decomposition.addPoint(time, 10.0);
}
}

LOG_DEBUG(<< "*** Trend and Seasonal Components ***");
{
std::string xml;
load("testfiles/CTimeSeriesDecomposition.5.6.trend_and_seasonal.state.xml", xml);
LOG_DEBUG(<< "Saved state size = " << xml.size());

core::CRapidXmlParser parser;
CPPUNIT_ASSERT(parser.parseStringIgnoreCdata(xml));
core::CRapidXmlStateRestoreTraverser traverser(parser);

maths::CTimeSeriesDecomposition decomposition(params, traverser);

// Check that the decay rates match and the values and variances
// predictions are close to the values obtained from 6.2. We can't
// update the state exactly in this case so the tolerances in this
// test are significantly larger.

CPPUNIT_ASSERT_EQUAL(0.024, decomposition.decayRate());

double meanValue{decomposition.meanValue(10366200)};
double meanVariance{decomposition.meanVariance()};
LOG_DEBUG(<< "restored mean value = " << meanValue);
LOG_DEBUG(<< "restored mean variance = " << meanVariance);
CPPUNIT_ASSERT_DOUBLES_EQUAL(96.5607, meanValue, 4.0);
CPPUNIT_ASSERT_DOUBLES_EQUAL(631.094, meanVariance, 7.0);

// Check some basic operations on the upgraded model.
decomposition.forecast(10366200, 10366200 + WEEK, HALF_HOUR, 90.0, 1.0,
[](core_t::TTime, const TDouble3Vec&) {});
for (core_t::TTime time = 60480000; time < 60480000 + WEEK; time += HALF_HOUR) {
decomposition.addPoint(time, 10.0);
}
}
}

CppUnit::Test* CTimeSeriesDecompositionTest::suite() {
Expand Down
Loading