From b209893dd050dec49b86f1ddea321c6136a07ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Monlla=C3=B3?= Date: Thu, 11 Jul 2019 16:10:48 +0200 Subject: [PATCH] MDL-65769 mlbackend_php: Proper algo init & unit test fix --- analytics/tests/privacy_test.php | 80 +++++++++++++++++++------ lib/mlbackend/php/classes/processor.php | 15 ++++- 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/analytics/tests/privacy_test.php b/analytics/tests/privacy_test.php index 8b93bc180d3..57f113b6452 100644 --- a/analytics/tests/privacy_test.php +++ b/analytics/tests/privacy_test.php @@ -70,6 +70,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2 = $this->getDataGenerator()->create_user(['firstname' => 'a222222222222', 'lastname' => 'a']); $this->u3 = $this->getDataGenerator()->create_user(['firstname' => 'b333333333333', 'lastname' => 'b']); $this->u4 = $this->getDataGenerator()->create_user(['firstname' => 'b444444444444', 'lastname' => 'b']); + $this->u5 = $this->getdatagenerator()->create_user(['firstname' => 'a555555555555', 'lastname' => 'a']); + $this->u6 = $this->getdatagenerator()->create_user(['firstname' => 'a666666666666', 'lastname' => 'a']); + $this->u7 = $this->getdatagenerator()->create_user(['firstname' => 'b777777777777', 'lastname' => 'b']); + $this->u8 = $this->getDataGenerator()->create_user(['firstname' => 'b888888888888', 'lastname' => 'b']); $this->c1 = $this->getDataGenerator()->create_course(['visible' => false]); $this->c2 = $this->getDataGenerator()->create_course(); @@ -78,10 +82,18 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->getDataGenerator()->enrol_user($this->u2->id, $this->c1->id, 'student'); $this->getDataGenerator()->enrol_user($this->u3->id, $this->c1->id, 'student'); $this->getDataGenerator()->enrol_user($this->u4->id, $this->c1->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u5->id, $this->c1->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u6->id, $this->c1->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u7->id, $this->c1->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u8->id, $this->c1->id, 'student'); $this->getDataGenerator()->enrol_user($this->u1->id, $this->c2->id, 'student'); $this->getDataGenerator()->enrol_user($this->u2->id, $this->c2->id, 'student'); $this->getDataGenerator()->enrol_user($this->u3->id, $this->c2->id, 'student'); $this->getDataGenerator()->enrol_user($this->u4->id, $this->c2->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u5->id, $this->c2->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u6->id, $this->c2->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u7->id, $this->c2->id, 'student'); + $this->getDataGenerator()->enrol_user($this->u8->id, $this->c2->id, 'student'); $this->setAdminUser(); @@ -111,7 +123,8 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $course1context = \context_course::instance($this->c1->id); $course2context = \context_course::instance($this->c2->id); $systemcontext = \context_system::instance(); - $expected = [$this->u1->id, $this->u2->id, $this->u3->id, $this->u4->id]; + $expected = [$this->u1->id, $this->u2->id, $this->u3->id, $this->u4->id, $this->u5->id, $this->u6->id, + $this->u7->id, $this->u8->id]; // Check users exist in the relevant contexts. $userlist = new \core_privacy\local\request\userlist($course1context, $component); @@ -145,9 +158,9 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider public function test_delete_context_data() { global $DB; - // We have 2 predictions for model1 and 4 predictions for model2. - $this->assertEquals(6, $DB->count_records('analytics_predictions')); - $this->assertEquals(14, $DB->count_records('analytics_indicator_calc')); + // We have 4 predictions for model1 and 8 predictions for model2. + $this->assertEquals(12, $DB->count_records('analytics_predictions')); + $this->assertEquals(26, $DB->count_records('analytics_indicator_calc')); // We have 1 prediction action. $this->assertEquals(1, $DB->count_records('analytics_prediction_actions')); @@ -157,8 +170,8 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider // Delete the course that was used for prediction. provider::delete_data_for_all_users_in_context($coursecontext); - // The course predictions are deleted. - $this->assertEquals(4, $DB->count_records('analytics_predictions')); + // The course1 predictions are deleted. + $this->assertEquals(8, $DB->count_records('analytics_predictions')); // Calculations related to that context are deleted. $this->assertEmpty($DB->count_records('analytics_indicator_calc', ['contextid' => $coursecontext->id])); @@ -181,7 +194,7 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider provider::delete_data_for_user($contextlist); // The site level prediction for u3 was deleted. - $this->assertEquals(3, $DB->count_records('analytics_predictions')); + $this->assertEquals(9, $DB->count_records('analytics_predictions')); $this->assertEquals(0, $DB->count_records('analytics_prediction_actions')); $usercontexts = provider::get_contexts_for_userid($this->u1->id); @@ -189,13 +202,13 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $usercontexts->get_contextids()); provider::delete_data_for_user($contextlist); // We have nothing for u1. - $this->assertEquals(3, $DB->count_records('analytics_predictions')); + $this->assertEquals(9, $DB->count_records('analytics_predictions')); $usercontexts = provider::get_contexts_for_userid($this->u4->id); $contextlist = new \core_privacy\local\request\approved_contextlist($this->u4, 'core_analytics', $usercontexts->get_contextids()); provider::delete_data_for_user($contextlist); - $this->assertEquals(0, $DB->count_records('analytics_predictions')); + $this->assertEquals(6, $DB->count_records('analytics_predictions')); } /** @@ -218,6 +231,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => provider::get_contexts_for_userid($this->u2->id)->get_contextids(), $this->u3->id => provider::get_contexts_for_userid($this->u3->id)->get_contextids(), $this->u4->id => provider::get_contexts_for_userid($this->u4->id)->get_contextids(), + $this->u5->id => provider::get_contexts_for_userid($this->u5->id)->get_contextids(), + $this->u6->id => provider::get_contexts_for_userid($this->u6->id)->get_contextids(), + $this->u7->id => provider::get_contexts_for_userid($this->u7->id)->get_contextids(), + $this->u8->id => provider::get_contexts_for_userid($this->u8->id)->get_contextids(), ]; foreach ($actualcontexts as $userid => $unused) { @@ -226,9 +243,9 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider } // Test initial record counts are as expected. - $this->assertEquals(6, $DB->count_records('analytics_predictions')); + $this->assertEquals(12, $DB->count_records('analytics_predictions')); $this->assertEquals(1, $DB->count_records('analytics_prediction_actions')); - $this->assertEquals(14, $DB->count_records('analytics_indicator_calc')); + $this->assertEquals(26, $DB->count_records('analytics_indicator_calc')); // Delete u1 and u3 from system context. $approveduserids = [$this->u1->id, $this->u3->id]; @@ -241,6 +258,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => [$systemcontext->id, $course1context->id, $course2context->id], $this->u3->id => [$course1context->id, $course2context->id], $this->u4->id => [$systemcontext->id, $course1context->id, $course2context->id], + $this->u5->id => [$systemcontext->id, $course1context->id, $course2context->id], + $this->u6->id => [$systemcontext->id, $course1context->id, $course2context->id], + $this->u7->id => [$systemcontext->id, $course1context->id, $course2context->id], + $this->u8->id => [$systemcontext->id, $course1context->id, $course2context->id], ]; $actualcontexts = [ @@ -248,6 +269,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => provider::get_contexts_for_userid($this->u2->id)->get_contextids(), $this->u3->id => provider::get_contexts_for_userid($this->u3->id)->get_contextids(), $this->u4->id => provider::get_contexts_for_userid($this->u4->id)->get_contextids(), + $this->u5->id => provider::get_contexts_for_userid($this->u5->id)->get_contextids(), + $this->u6->id => provider::get_contexts_for_userid($this->u6->id)->get_contextids(), + $this->u7->id => provider::get_contexts_for_userid($this->u7->id)->get_contextids(), + $this->u8->id => provider::get_contexts_for_userid($this->u8->id)->get_contextids(), ]; foreach ($actualcontexts as $userid => $unused) { @@ -257,12 +282,13 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider } // Test expected number of records have been deleted. - $this->assertEquals(5, $DB->count_records('analytics_predictions')); + $this->assertEquals(11, $DB->count_records('analytics_predictions')); $this->assertEquals(1, $DB->count_records('analytics_prediction_actions')); - $this->assertEquals(12, $DB->count_records('analytics_indicator_calc')); + $this->assertEquals(24, $DB->count_records('analytics_indicator_calc')); - // Delete for all 4 users in course 2 context. - $approveduserids = [$this->u1->id, $this->u2->id, $this->u3->id, $this->u4->id]; + // Delete for all 8 users in course 2 context. + $approveduserids = [$this->u1->id, $this->u2->id, $this->u3->id, $this->u4->id, $this->u5->id, $this->u6->id, + $this->u7->id, $this->u8->id]; $approvedlist = new approved_userlist($course2context, $component, $approveduserids); provider::delete_data_for_users($approvedlist); @@ -272,6 +298,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => [$systemcontext->id, $course1context->id], $this->u3->id => [$course1context->id], $this->u4->id => [$systemcontext->id, $course1context->id], + $this->u5->id => [$systemcontext->id, $course1context->id], + $this->u6->id => [$systemcontext->id, $course1context->id], + $this->u7->id => [$systemcontext->id, $course1context->id], + $this->u8->id => [$systemcontext->id, $course1context->id], ]; $actualcontexts = [ @@ -279,6 +309,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => provider::get_contexts_for_userid($this->u2->id)->get_contextids(), $this->u3->id => provider::get_contexts_for_userid($this->u3->id)->get_contextids(), $this->u4->id => provider::get_contexts_for_userid($this->u4->id)->get_contextids(), + $this->u5->id => provider::get_contexts_for_userid($this->u5->id)->get_contextids(), + $this->u6->id => provider::get_contexts_for_userid($this->u6->id)->get_contextids(), + $this->u7->id => provider::get_contexts_for_userid($this->u7->id)->get_contextids(), + $this->u8->id => provider::get_contexts_for_userid($this->u8->id)->get_contextids(), ]; foreach ($actualcontexts as $userid => $unused) { @@ -288,9 +322,9 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider } // Test expected number of records have been deleted. - $this->assertEquals(3, $DB->count_records('analytics_predictions')); + $this->assertEquals(7, $DB->count_records('analytics_predictions')); $this->assertEquals(1, $DB->count_records('analytics_prediction_actions')); - $this->assertEquals(8, $DB->count_records('analytics_indicator_calc')); + $this->assertEquals(16, $DB->count_records('analytics_indicator_calc')); $approveduserids = [$this->u3->id]; $approvedlist = new approved_userlist($course1context, $component, $approveduserids); @@ -302,6 +336,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => [$systemcontext->id, $course1context->id], $this->u3->id => [], $this->u4->id => [$systemcontext->id, $course1context->id], + $this->u5->id => [$systemcontext->id, $course1context->id], + $this->u6->id => [$systemcontext->id, $course1context->id], + $this->u7->id => [$systemcontext->id, $course1context->id], + $this->u8->id => [$systemcontext->id, $course1context->id], ]; $actualcontexts = [ @@ -309,6 +347,10 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider $this->u2->id => provider::get_contexts_for_userid($this->u2->id)->get_contextids(), $this->u3->id => provider::get_contexts_for_userid($this->u3->id)->get_contextids(), $this->u4->id => provider::get_contexts_for_userid($this->u4->id)->get_contextids(), + $this->u5->id => provider::get_contexts_for_userid($this->u5->id)->get_contextids(), + $this->u6->id => provider::get_contexts_for_userid($this->u6->id)->get_contextids(), + $this->u7->id => provider::get_contexts_for_userid($this->u7->id)->get_contextids(), + $this->u8->id => provider::get_contexts_for_userid($this->u8->id)->get_contextids(), ]; foreach ($actualcontexts as $userid => $unused) { sort($actualcontexts[$userid]); @@ -317,9 +359,9 @@ class core_analytics_privacy_model_testcase extends \core_privacy\tests\provider } // Test expected number of records have been deleted. - $this->assertEquals(2, $DB->count_records('analytics_predictions')); + $this->assertEquals(6, $DB->count_records('analytics_predictions')); $this->assertEquals(0, $DB->count_records('analytics_prediction_actions')); - $this->assertEquals(7, $DB->count_records('analytics_indicator_calc')); + $this->assertEquals(15, $DB->count_records('analytics_indicator_calc')); } /** diff --git a/lib/mlbackend/php/classes/processor.php b/lib/mlbackend/php/classes/processor.php index 063c44db96d..b9b122cfcad 100644 --- a/lib/mlbackend/php/classes/processor.php +++ b/lib/mlbackend/php/classes/processor.php @@ -30,6 +30,7 @@ use Phpml\Preprocessing\Normalizer; use Phpml\CrossValidation\RandomSplit; use Phpml\Dataset\ArrayDataset; use Phpml\ModelManager; +use Phpml\Classification\Linear\LogisticRegression; /** * PHP predictions processor. @@ -110,7 +111,7 @@ class processor implements \core_analytics\classifier, \core_analytics\regressor if (file_exists($modelfilepath)) { $classifier = $modelmanager->restoreFromFile($modelfilepath); } else { - $classifier = new \Phpml\Classification\Linear\LogisticRegression(self::TRAIN_ITERATIONS, Normalizer::NORM_L2); + $classifier = $this->instantiate_algorithm(); } $fh = $dataset->get_content_file_handle(); @@ -314,7 +315,7 @@ class processor implements \core_analytics\classifier, \core_analytics\regressor for ($i = 0; $i < $niterations; $i++) { if (!$trainedmodeldir) { - $classifier = new \Phpml\Classification\Linear\LogisticRegression(self::TRAIN_ITERATIONS, Normalizer::NORM_L2); + $classifier = $this->instantiate_algorithm(); // Split up the dataset in classifier and testing. $data = new RandomSplit(new ArrayDataset($samples, $targets), 0.2); @@ -561,4 +562,14 @@ class processor implements \core_analytics\classifier, \core_analytics\regressor $metadata = fgetcsv($fh); return array_combine($metadata, fgetcsv($fh)); } + + /** + * Instantiates the ML algorithm. + * + * @return \Phpml\Classification\Linear\LogisticRegression + */ + protected function instantiate_algorithm(): \Phpml\Classification\Linear\LogisticRegression { + return new LogisticRegression(self::TRAIN_ITERATIONS, true, + LogisticRegression::CONJUGATE_GRAD_TRAINING, 'log'); + } }