MDL-60003 analytics: Skip samples with only null values
authorDavid Monllao <davidm@moodle.com>
Tue, 5 Sep 2017 05:06:30 +0000 (07:06 +0200)
committerDavid Monllao <davidm@moodle.com>
Thu, 28 Sep 2017 14:56:33 +0000 (16:56 +0200)
analytics/classes/local/indicator/base.php
analytics/classes/local/time_splitting/base.php
analytics/tests/prediction_test.php

index 7c6aa3a..5cd88b9 100644 (file)
@@ -147,7 +147,7 @@ abstract class base extends \core_analytics\calculable {
      * @param integer $starttime Limit the calculation to this timestart
      * @param integer $endtime Limit the calculation to this timeend
      * @param array $existingcalculations Existing calculations of this indicator, indexed by sampleid.
-     * @return array array[0] with format [sampleid] = int[]|float[], array[1] with format [sampleid] = int|float
+     * @return array [0] = [$sampleid => int[]|float[]], [1] = [$sampleid => int|float], [2] = [$sampleid => $sampleid]
      */
     public function calculate($sampleids, $samplesorigin, $starttime = false, $endtime = false, $existingcalculations = array()) {
 
@@ -157,6 +157,7 @@ abstract class base extends \core_analytics\calculable {
 
         $calculations = array();
         $newcalculations = array();
+        $notnulls = array();
         foreach ($sampleids as $sampleid => $unusedsampleid) {
 
             if (isset($existingcalculations[$sampleid])) {
@@ -166,9 +167,12 @@ abstract class base extends \core_analytics\calculable {
                 $newcalculations[$sampleid] = $calculatedvalue;
             }
 
-            if (!is_null($calculatedvalue) && ($calculatedvalue > self::MAX_VALUE || $calculatedvalue < self::MIN_VALUE)) {
-                throw new \coding_exception('Calculated values should be higher than ' . self::MIN_VALUE .
-                    ' and lower than ' . self::MAX_VALUE . ' ' . $calculatedvalue . ' received');
+            if (!is_null($calculatedvalue)) {
+                $notnulls[$sampleid] = $sampleid;
+                if ($calculatedvalue > self::MAX_VALUE || $calculatedvalue < self::MIN_VALUE) {
+                    throw new \coding_exception('Calculated values should be higher than ' . self::MIN_VALUE .
+                        ' and lower than ' . self::MAX_VALUE . ' ' . $calculatedvalue . ' received');
+                }
             }
 
             $calculations[$sampleid] = $calculatedvalue;
@@ -176,6 +180,6 @@ abstract class base extends \core_analytics\calculable {
 
         $features = $this->to_features($calculations);
 
-        return array($features, $newcalculations);
+        return array($features, $newcalculations, $notnulls);
     }
 }
index f45934a..11532a9 100644 (file)
@@ -231,6 +231,9 @@ abstract class base {
                 $range['start'], $range['end'], $samplesorigin);
         }
 
+        // Here we store samples which calculations are not all null.
+        $notnulls = array();
+
         // Fill the dataset samples with indicators data.
         $newcalculations = array();
         foreach ($indicators as $indicator) {
@@ -250,7 +253,7 @@ abstract class base {
                 }
 
                 // Calculate the indicator for each sample in this time range.
-                list($samplesfeatures, $newindicatorcalculations) = $rangeindicator->calculate($sampleids,
+                list($samplesfeatures, $newindicatorcalculations, $indicatornotnulls) = $rangeindicator->calculate($sampleids,
                     $samplesorigin, $range['start'], $range['end'], $prevcalculations);
 
                 // Copy the features data to the dataset.
@@ -258,6 +261,10 @@ abstract class base {
 
                     $uniquesampleid = $this->append_rangeindex($analysersampleid, $rangeindex);
 
+                    if (!isset($notnulls[$uniquesampleid]) && !empty($indicatornotnulls[$analysersampleid])) {
+                        $notnulls[$uniquesampleid] = $uniquesampleid;
+                    }
+
                     // Init the sample if it is still empty.
                     if (!isset($dataset[$uniquesampleid])) {
                         $dataset[$uniquesampleid] = array();
@@ -307,6 +314,15 @@ abstract class base {
             $DB->insert_records('analytics_indicator_calc', $newcalculations);
         }
 
+        // Delete rows where all calculations are null.
+        // We still store the indicator calculation and we still store the sample id as
+        // processed so we don't have to process this sample again, but we exclude it
+        // from the dataset because it is not useful.
+        $nulls = array_diff_key($dataset, $notnulls);
+        foreach ($nulls as $uniqueid => $ignoredvalues) {
+            unset($dataset[$uniqueid]);
+        }
+
         return $dataset;
     }
 
index 1e21d8d..7f30037 100644 (file)
@@ -361,6 +361,57 @@ class core_analytics_prediction_testcase extends advanced_testcase {
         list($values, $unused) = $indicator->calculate($sampleids, $sampleorigin, $starttime, $endtime, $existingcalcs);
     }
 
+    /**
+     * test_not_null_samples
+     */
+    public function test_not_null_samples() {
+        $this->resetAfterTest(true);
+
+        $classname = '\core\analytics\time_splitting\quarters';
+        $timesplitting = \core_analytics\manager::get_time_splitting($classname);
+        $timesplitting->set_analysable(new \core_analytics\site());
+
+        $ranges = array(
+            array('start' => 111, 'end' => 222, 'time' => 222),
+            array('start' => 222, 'end' => 333, 'time' => 333)
+        );
+        $samples = array(123 => 123, 321 => 321);
+
+        $indicator1 = $this->getMockBuilder('test_indicator_max')
+            ->setMethods(['calculate_sample'])
+            ->getMock();
+        $indicator1->method('calculate_sample')
+            ->willReturn(null);
+
+        $indicator2 = \core_analytics\manager::get_indicator('test_indicator_min');
+
+        // Samples with at least 1 not null value are returned.
+        $params = array(
+            $samples,
+            'whatever',
+            array($indicator1, $indicator2),
+            $ranges
+        );
+        $dataset = phpunit_util::call_internal_method($timesplitting, 'calculate_indicators', $params, $classname);
+        $this->assertArrayHasKey('123-0', $dataset);
+        $this->assertArrayHasKey('123-1', $dataset);
+        $this->assertArrayHasKey('321-0', $dataset);
+        $this->assertArrayHasKey('321-1', $dataset);
+
+        // Samples with only null values are not returned.
+        $params = array(
+            $samples,
+            'whatever',
+            array($indicator1),
+            $ranges
+        );
+        $dataset = phpunit_util::call_internal_method($timesplitting, 'calculate_indicators', $params, $classname);
+        $this->assertArrayNotHasKey('123-0', $dataset);
+        $this->assertArrayNotHasKey('123-1', $dataset);
+        $this->assertArrayNotHasKey('321-0', $dataset);
+        $this->assertArrayNotHasKey('321-1', $dataset);
+    }
+
     /**
      * provider_ml_test_evaluation
      *