diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index 6988b769b3..1a0c694737 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -28,6 +28,22 @@ //=== Regressions + == {es} version 6.6.0 + +=== Breaking Changes + +=== Deprecations + +=== New Features + +=== Enhancements + +=== Bug Fixes + +Fix cause of "Sample out of bounds" error message (See {ml-pull}355[355].} + +=== Regressions + == {es} version 6.5.3 === Bug Fixes diff --git a/lib/maths/CPoissonMeanConjugate.cc b/lib/maths/CPoissonMeanConjugate.cc index e31cc73ee0..0ee21000db 100644 --- a/lib/maths/CPoissonMeanConjugate.cc +++ b/lib/maths/CPoissonMeanConjugate.cc @@ -646,6 +646,9 @@ void CPoissonMeanConjugate::sampleMarginalLikelihood(std::size_t numberSamples, // Sanity check the sample: should be in the distribution support. if (sample >= support.first && sample <= support.second) { samples.push_back(sample); + } else if (sample < support.first) { + // Extremely rarely the variance is large and the offset is 0, resulting in a negative sample. + // We discard these negative samples as they are logically incongruous for a counting distribution. } else { LOG_ERROR(<< "Sample out of bounds: sample = " << sample << ", support = [" << support.first << "," << support.second << "]" diff --git a/lib/maths/unittest/CPoissonMeanConjugateTest.cc b/lib/maths/unittest/CPoissonMeanConjugateTest.cc index 77a4af4e3a..eb8c126c1b 100644 --- a/lib/maths/unittest/CPoissonMeanConjugateTest.cc +++ b/lib/maths/unittest/CPoissonMeanConjugateTest.cc @@ -494,6 +494,54 @@ void CPoissonMeanConjugateTest::testSampleMarginalLikelihood() { } } +void CPoissonMeanConjugateTest::testSampleMarginalLikelihoodInSupportBounds() { + // Here we test that the retrieved samples are + // a) positive + // b) monotonically increasing + // c) No ERROR log messages are generated + // The input sample and weight are chosen to be ones that generate + // an initial sample that is less than an offset value of 0.0 + + const char* logFile = "test.log"; + + std::remove(logFile); + // log at level ERROR only + CPPUNIT_ASSERT(ml::core::CLogger::instance().reconfigureFromFile( + "testfiles/testLogErrorsLog4cxx.properties")); + + CPoissonMeanConjugate filter(CPoissonMeanConjugate::nonInformativePrior()); + + TDouble1Vec sampleVec(1, 283.3232); + maths_t::TDoubleWeightsAry1Vec weightVec(1, maths_t::countWeight(0.0206505)); + + filter.addSamples(sampleVec, weightVec); + + std::size_t numberSampled = 50u; + TDouble1Vec sampled; + + filter.sampleMarginalLikelihood(numberSampled, sampled); + + double prevSample{0.0}; + for (double sample : sampled) { + CPPUNIT_ASSERT(sample > prevSample); + CPPUNIT_ASSERT(sample >= 0.0); + prevSample = sample; + } + + // Revert to the default properties for the test framework - very similar to the hardcoded default. + CPPUNIT_ASSERT(ml::core::CLogger::instance().reconfigureFromFile("testfiles/log4cxx.properties")); + + std::ifstream log(logFile); + CPPUNIT_ASSERT(log.is_open()); + char line[256]; + while (log.getline(line, 256)) { + LOG_INFO(<< "Got '" << line << "'"); + CPPUNIT_ASSERT(false); + } + log.close(); + std::remove(logFile); +} + void CPoissonMeanConjugateTest::testCdf() { // Test error cases. // @@ -960,6 +1008,9 @@ CppUnit::Test* CPoissonMeanConjugateTest::suite() { &CPoissonMeanConjugateTest::testSampleMarginalLikelihood)); suiteOfTests->addTest(new CppUnit::TestCaller( "CPoissonMeanConjugateTest::testCdf", &CPoissonMeanConjugateTest::testCdf)); + suiteOfTests->addTest(new CppUnit::TestCaller( + "CPoissonMeanConjugateTest::testSampleMarginalLikelihoodInSupportBounds", + &CPoissonMeanConjugateTest::testSampleMarginalLikelihoodInSupportBounds)); suiteOfTests->addTest(new CppUnit::TestCaller( "CPoissonMeanConjugateTest::testProbabilityOfLessLikelySamples", &CPoissonMeanConjugateTest::testProbabilityOfLessLikelySamples)); diff --git a/lib/maths/unittest/CPoissonMeanConjugateTest.h b/lib/maths/unittest/CPoissonMeanConjugateTest.h index 8a302249f9..30955c7a9e 100644 --- a/lib/maths/unittest/CPoissonMeanConjugateTest.h +++ b/lib/maths/unittest/CPoissonMeanConjugateTest.h @@ -15,6 +15,7 @@ class CPoissonMeanConjugateTest : public CppUnit::TestFixture { void testPropagation(); void testMeanEstimation(); void testMarginalLikelihood(); + void testSampleMarginalLikelihoodInSupportBounds(); void testMarginalLikelihoodMode(); void testMarginalLikelihoodVariance(); void testSampleMarginalLikelihood(); diff --git a/lib/maths/unittest/testfiles/log4cxx.properties b/lib/maths/unittest/testfiles/log4cxx.properties new file mode 100644 index 0000000000..2d36596651 --- /dev/null +++ b/lib/maths/unittest/testfiles/log4cxx.properties @@ -0,0 +1,10 @@ +# Set root logger level to DEBUG and its only appender to A1. +log4j.rootLogger=DEBUG, A1 + +# A1 is set to be a ConsoleAppender. +log4j.appender.A1=org.apache.log4j.ConsoleAppender +log4j.appender.A1.Target=System.err + +# A1 uses PatternLayout. +log4j.appender.A1.layout=org.apache.log4j.PatternLayout +log4j.appender.A1.layout.ConversionPattern=%d %d{%Z} [%P] %-5p %F@%L %m%n diff --git a/lib/maths/unittest/testfiles/testLogErrorsLog4cxx.properties b/lib/maths/unittest/testfiles/testLogErrorsLog4cxx.properties new file mode 100644 index 0000000000..0a9e7726ee --- /dev/null +++ b/lib/maths/unittest/testfiles/testLogErrorsLog4cxx.properties @@ -0,0 +1,20 @@ +# Set root logger level to INFO and its only appender to A1. +log4j.rootLogger=DEBUG, A1 + +# Also set up a logger named after the log identifier of the program, with +# appender A2. +log4j.logger.%N=ERROR, A2 + +# A1 is set to be a ConsoleAppender. +log4j.appender.A1=org.apache.log4j.ConsoleAppender +log4j.appender.A1.Target=System.err +log4j.appender.A1.layout=org.apache.log4j.PatternLayout +log4j.appender.A1.layout.ConversionPattern=%d %d{%Z} [%P] %-5p %F@%L %m%n + +# A2 is set to log to a rolling file in a current directory. +log4j.appender.A2=org.apache.log4j.RollingFileAppender +log4j.appender.A2.File=test.log +log4j.appender.A2.MaxFileSize=10MB +log4j.appender.A2.MaxBackupIndex=1 +log4j.appender.A2.layout=org.apache.log4j.PatternLayout +log4j.appender.A2.layout.ConversionPattern=%d %d{%Z} [%P] %-5p %F@%L %m%n