From 1611308b5834812d88994add151f1ea47102ad75 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Thu, 15 Jun 2017 10:21:58 +0200 Subject: [PATCH] MDL-57791 analytics: Changes after review - Split model::predict in parts - JS promises updated according to eslint-plugin-promise - New API methods replacing direct DB queries - Reduce insights nav link display cost - Increase time limit as well as memory for big processes - Move prediction action event to core - Dataset write locking and others - Refine last time range end time - Removed dodgy splitting method id to int - Replace admin_setting_predictor output_html overwrite for write_setting overwrite - New APIs for access control - Discard invalid samples also during prediction --- admin/settings/analytics.php | 2 +- admin/tool/models/amd/build/log_info.min.js | 2 +- admin/tool/models/amd/src/log_info.js | 10 +- .../analytics/target/course_dropout.php | 9 +- .../classes/analytics/target/no_teaching.php | 7 +- .../tool/models/classes/output/model_logs.php | 2 - .../models/classes/task/predict_models.php | 2 +- .../tool/models/classes/task/train_models.php | 2 +- admin/tool/models/cli/enable_model.php | 3 +- admin/tool/models/cli/evaluate_model.php | 3 +- admin/tool/models/model.php | 2 +- analytics/classes/admin_setting_predictor.php | 26 +- analytics/classes/calculable.php | 12 +- analytics/classes/course.php | 6 - analytics/classes/dataset_manager.php | 47 ++-- analytics/classes/local/analyser/base.php | 29 ++- .../community_of_inquiry_activity.php | 5 + .../local/indicator/user_track_forums.php | 3 - analytics/classes/local/target/base.php | 122 ++++++--- .../classes/local/time_splitting/base.php | 2 +- .../classes/local/time_splitting/deciles.php | 3 + .../local/time_splitting/deciles_accum.php | 3 + .../classes/local/time_splitting/quarters.php | 4 +- .../local/time_splitting/quarters_accum.php | 4 +- analytics/classes/manager.php | 117 ++++++++- analytics/classes/model.php | 231 ++++++++++++------ .../tests/fixtures/test_target_shortname.php | 11 +- analytics/tests/model_test.php | 2 + lang/en/analytics.php | 8 +- lang/en/cache.php | 2 +- .../event/prediction_action_started.php | 18 +- lib/db/caches.php | 4 +- lib/mlbackend/php/classes/processor.php | 2 +- .../python/lang/en/mlbackend_python.php | 2 +- report/insights/action.php | 30 +-- report/insights/classes/output/insight.php | 12 +- .../insights/classes/output/insights_list.php | 2 +- report/insights/insights.php | 17 +- report/insights/lib.php | 6 +- report/insights/prediction.php | 28 +-- 40 files changed, 513 insertions(+), 289 deletions(-) rename analytics/classes/event/action_clicked.php => lib/classes/event/prediction_action_started.php (77%) diff --git a/admin/settings/analytics.php b/admin/settings/analytics.php index 32a26d9cbeb..f3dfa2091cb 100644 --- a/admin/settings/analytics.php +++ b/admin/settings/analytics.php @@ -45,7 +45,7 @@ if ($hassiteconfig) { $logmanager = get_log_manager(); $readers = $logmanager->get_readers('core\log\sql_reader'); $options = array(); - $defaultreader = false; + $defaultreader = null; foreach ($readers as $plugin => $reader) { if (!$reader->is_logging()) { continue; diff --git a/admin/tool/models/amd/build/log_info.min.js b/admin/tool/models/amd/build/log_info.min.js index 39e47e77c56..c4d0a9e2da9 100644 --- a/admin/tool/models/amd/build/log_info.min.js +++ b/admin/tool/models/amd/build/log_info.min.js @@ -1 +1 @@ -define(["jquery","core/str","core/modal_factory"],function(a,b,c){return{loadInfo:function(d,e){var f=a('[data-model-log-id="'+d+'"]');b.get_string("loginfo","tool_models").done(function(b){var d=a(""),c.create({title:b,body:d.html(),large:!0},f)})}}}); \ No newline at end of file +define(["jquery","core/str","core/modal_factory","core/notification"],function(a,b,c,d){return{loadInfo:function(e,f){var g=a('[data-model-log-id="'+e+'"]');b.get_string("loginfo","tool_models").then(function(b){var d=a(""),c.create({title:b,body:d.html(),large:!0},g)})["catch"](d.exception)}}}); \ No newline at end of file diff --git a/admin/tool/models/amd/src/log_info.js b/admin/tool/models/amd/src/log_info.js index a5799c92736..5af6d6aaa4b 100644 --- a/admin/tool/models/amd/src/log_info.js +++ b/admin/tool/models/amd/src/log_info.js @@ -7,7 +7,7 @@ /** * @module tool_models/log_info */ -define(['jquery', 'core/str', 'core/modal_factory'], function($, str, ModalFactory) { +define(['jquery', 'core/str', 'core/modal_factory', 'core/notification'], function($, str, ModalFactory, Notification) { return { @@ -20,19 +20,21 @@ define(['jquery', 'core/str', 'core/modal_factory'], function($, str, ModalFacto loadInfo : function(id, info) { var link = $('[data-model-log-id="' + id + '"]'); - str.get_string('loginfo', 'tool_models').done(function(langString) { + str.get_string('loginfo', 'tool_models').then(function(langString) { var bodyInfo = $(""); - ModalFactory.create({ + + return ModalFactory.create({ title: langString, body: bodyInfo.html(), large: true, }, link); - }); + + }).catch(Notification.exception); } }; }); diff --git a/admin/tool/models/classes/analytics/target/course_dropout.php b/admin/tool/models/classes/analytics/target/course_dropout.php index 8e0e7ec6a3d..1f6554f224e 100644 --- a/admin/tool/models/classes/analytics/target/course_dropout.php +++ b/admin/tool/models/classes/analytics/target/course_dropout.php @@ -46,10 +46,10 @@ class course_dropout extends \core_analytics\local\target\binary { return get_string('target:coursedropout', 'tool_models'); } - public function prediction_actions(\core_analytics\prediction $prediction) { + public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false) { global $USER; - $actions = parent::prediction_actions($prediction); + $actions = parent::prediction_actions($prediction, $includedetailsaction); $sampledata = $prediction->get_sample_data(); $studentid = $sampledata['user']->id; @@ -140,13 +140,14 @@ class course_dropout extends \core_analytics\local\target\binary { } /** - * is_valid_sample + * All student enrolments are valid. * * @param int $sampleid * @param \core_analytics\analysable $course + * @param bool $fortraining * @return bool */ - public function is_valid_sample($sampleid, \core_analytics\analysable $course) { + public function is_valid_sample($sampleid, \core_analytics\analysable $course, $fortraining = true) { return true; } diff --git a/admin/tool/models/classes/analytics/target/no_teaching.php b/admin/tool/models/classes/analytics/target/no_teaching.php index eaff6117c41..8c0194c2cc1 100644 --- a/admin/tool/models/classes/analytics/target/no_teaching.php +++ b/admin/tool/models/classes/analytics/target/no_teaching.php @@ -48,10 +48,10 @@ class no_teaching extends \core_analytics\local\target\binary { return get_string('target:noteachingactivity', 'tool_models'); } - public function prediction_actions(\core_analytics\prediction $prediction) { + public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false) { global $USER; - // No need to call the parent as the only default action is view details and this target only have 1 feature. + // No need to call the parent as the parent's action is view details and this target only have 1 feature. $actions = array(); $sampledata = $prediction->get_sample_data(); @@ -110,9 +110,10 @@ class no_teaching extends \core_analytics\local\target\binary { * * @param mixed $sampleid * @param \core_analytics\analysable $analysable + * @param bool $fortraining * @return void */ - public function is_valid_sample($sampleid, \core_analytics\analysable $analysable) { + public function is_valid_sample($sampleid, \core_analytics\analysable $analysable, $fortraining = true) { $course = $this->retrieve('course', $sampleid); diff --git a/admin/tool/models/classes/output/model_logs.php b/admin/tool/models/classes/output/model_logs.php index 059727ff1a9..e86eca5e68f 100644 --- a/admin/tool/models/classes/output/model_logs.php +++ b/admin/tool/models/classes/output/model_logs.php @@ -180,8 +180,6 @@ class model_logs extends \table_sql { * @param bool $useinitialsbar do you want to use the initials bar. */ public function query_db($pagesize, $useinitialsbar = true) { - global $DB; - $total = count($this->model->get_logs()); $this->pagesize($pagesize, $total); $this->rawdata = $this->model->get_logs($this->get_page_start(), $this->get_page_size()); diff --git a/admin/tool/models/classes/task/predict_models.php b/admin/tool/models/classes/task/predict_models.php index d6eb4ebef3d..366b8b6c8b8 100644 --- a/admin/tool/models/classes/task/predict_models.php +++ b/admin/tool/models/classes/task/predict_models.php @@ -39,7 +39,7 @@ class predict_models extends \core\task\scheduled_task { } public function execute() { - global $DB, $OUTPUT, $PAGE; + global $OUTPUT, $PAGE; $models = \core_analytics\manager::get_all_models(true, true); if (!$models) { diff --git a/admin/tool/models/classes/task/train_models.php b/admin/tool/models/classes/task/train_models.php index e8fd027aed2..cbe1f2e83ef 100644 --- a/admin/tool/models/classes/task/train_models.php +++ b/admin/tool/models/classes/task/train_models.php @@ -39,7 +39,7 @@ class train_models extends \core\task\scheduled_task { } public function execute() { - global $DB, $OUTPUT, $PAGE; + global $OUTPUT, $PAGE; $models = \core_analytics\manager::get_all_models(true); if (!$models) { diff --git a/admin/tool/models/cli/enable_model.php b/admin/tool/models/cli/enable_model.php index 13ceca8959b..59680d99423 100644 --- a/admin/tool/models/cli/enable_model.php +++ b/admin/tool/models/cli/enable_model.php @@ -63,8 +63,7 @@ if ($options['modelid'] === false || $options['timesplitting'] === false) { // We need admin permissions. \core\session\manager::set_user(get_admin()); -$modelobj = $DB->get_record('analytics_models', array('id' => $options['modelid']), '*', MUST_EXIST); -$model = new \core_analytics\model($modelobj); +$model = new \core_analytics\model($options['modelid']); // Evaluate its suitability to predict accurately. $model->enable($options['timesplitting']); diff --git a/admin/tool/models/cli/evaluate_model.php b/admin/tool/models/cli/evaluate_model.php index 7f342fa5da5..0036105440f 100644 --- a/admin/tool/models/cli/evaluate_model.php +++ b/admin/tool/models/cli/evaluate_model.php @@ -75,8 +75,7 @@ if ($options['filter'] !== false) { // We need admin permissions. \core\session\manager::set_user(get_admin()); -$modelobj = $DB->get_record('analytics_models', array('id' => $options['modelid']), '*', MUST_EXIST); -$model = new \core_analytics\model($modelobj); +$model = new \core_analytics\model($options['modelid']); mtrace(get_string('analysingsitedata', 'tool_models')); diff --git a/admin/tool/models/model.php b/admin/tool/models/model.php index 8d15d315237..676585b8b13 100644 --- a/admin/tool/models/model.php +++ b/admin/tool/models/model.php @@ -30,9 +30,9 @@ $action = required_param('action', PARAM_ALPHANUMEXT); $context = context_system::instance(); require_login(); -require_capability('moodle/analytics:managemodels', $context); $model = new \core_analytics\model($id); +\core_analytics\manager::check_can_manage_models(); $params = array('id' => $id, 'action' => $action); $url = new \moodle_url('/admin/tool/models/model.php', $params); diff --git a/analytics/classes/admin_setting_predictor.php b/analytics/classes/admin_setting_predictor.php index cf2be8b4199..e1b0b6aace3 100644 --- a/analytics/classes/admin_setting_predictor.php +++ b/analytics/classes/admin_setting_predictor.php @@ -30,28 +30,26 @@ require_once(__DIR__ . '/../../lib/adminlib.php'); class admin_setting_predictor extends \admin_setting_configselect { /** - * Builds HTML to display the control. + * Save a setting * - * The main purpose of this is to display a warning if the selected predictions processor is not ready. - - * @param string $data Unused - * @param string $query - * @return string HTML + * @param string $data + * @return string empty of error string */ - public function output_html($data, $query='') { - global $CFG, $OUTPUT; - - $html = ''; + public function write_setting($data) { + if (!$this->load_choices() or empty($this->choices)) { + return ''; + } + if (!array_key_exists($data, $this->choices)) { + return ''; // ignore it + } // Calling it here without checking if it is ready because we check it below and show it as a controlled case. $selectedprocessor = \core_analytics\manager::get_predictions_processor($data, false); - $isready = $selectedprocessor->is_ready(); if ($isready !== true) { - $html .= $OUTPUT->notification(get_string('errorprocessornotready', 'analytics', $isready)); + return get_string('errorprocessornotready', 'analytics', $isready); } - $html .= parent::output_html($data, $query); - return $html; + return ($this->config_write($this->name, $data) ? '' : get_string('errorsetting', 'admin')); } } diff --git a/analytics/classes/calculable.php b/analytics/classes/calculable.php index 5559fcfe97a..29147b76f89 100644 --- a/analytics/classes/calculable.php +++ b/analytics/classes/calculable.php @@ -130,7 +130,8 @@ abstract class calculable { /** * Returns the number of weeks a time range contains. * - * Useful for calculations that depend on the time range duration. + * Useful for calculations that depend on the time range duration. Note that it returns + * a float, rounding the float may lead to inaccurate results. * * @param int $starttime * @param int $endtime @@ -141,9 +142,14 @@ abstract class calculable { throw new \coding_exception('End time timestamp should be greater than start time.'); } - $diff = $endtime - $starttime; + $starttimedt = new \DateTime(); + $starttimedt->setTimestamp($starttime); + $starttimedt->setTimezone(\DateTimeZone::UTC); + $endtimedt = new \DateTime(); + $endtimedt->setTimestamp($endtime); + $endtimedt->setTimezone(\DateTimeZone::UTC); - // No need to be strict about DST here. + $diff = $endtimedt->getTimestamp() - $starttimedt->getTimestamp(); return $diff / WEEKSECS; } diff --git a/analytics/classes/course.php b/analytics/classes/course.php index 5a66179fc8a..2d8ee540dc7 100644 --- a/analytics/classes/course.php +++ b/analytics/classes/course.php @@ -443,9 +443,6 @@ class course implements \core_analytics\analysable { return false; } - // TODO Use course_modules_completion's timemodified + COMPLETION_COMPLETE* to discard - // activities that have already been completed. - // We skip activities that were not yet visible or their 'until' was not in this $starttime - $endtime range. if ($activity->availability) { $info = new \core_availability\info_module($activity); @@ -485,7 +482,6 @@ class course implements \core_analytics\analysable { } } - // TODO Think about activities in sectionnum 0. if ($activity->sectionnum == 0) { return false; } @@ -533,8 +529,6 @@ class course implements \core_analytics\analysable { $dateconditions = $info->get_availability_tree()->get_all_children('\availability_date\condition'); foreach ($dateconditions as $condition) { // Availability API does not allow us to check from / to dates nicely, we need to be naughty. - // TODO Would be nice to expand \availability_date\condition API for this calling a save that - // does not save is weird. $conditiondata = $condition->save(); if ($conditiondata->d === \availability_date\condition::DIRECTION_FROM && diff --git a/analytics/classes/dataset_manager.php b/analytics/classes/dataset_manager.php index 676b2c199db..115f1cbd824 100644 --- a/analytics/classes/dataset_manager.php +++ b/analytics/classes/dataset_manager.php @@ -86,18 +86,21 @@ class dataset_manager { /** * Mark the analysable as being analysed. * - * @return void + * @return bool Could we get the lock or not. */ public function init_process() { $lockkey = 'modelid:' . $this->modelid . '-analysableid:' . $this->analysableid . - '-timesplitting:' . self::convert_to_int($this->timesplittingid) . '-includetarget:' . (int)$this->includetarget; + '-timesplitting:' . self::clean_time_splitting_id($this->timesplittingid) . '-includetarget:' . (int)$this->includetarget; // Large timeout as processes may be quite long. $lockfactory = \core\lock\lock_config::get_lock_factory('core_analytics'); - $this->lock = $lockfactory->get_lock($lockkey, WEEKSECS); - // We release the lock if there is an error during the process. - \core_shutdown_manager::register_function(array($this, 'release_lock'), array($this->lock)); + // If it is not ready in 10 secs skip this model + analysable + timesplittingmethod combination + // it will attempt it again during next cron run. + if (!$this->lock = $lockfactory->get_lock($lockkey, 10)) { + return false; + } + return true; } /** @@ -115,7 +118,7 @@ class dataset_manager { 'filearea' => self::get_filearea($this->includetarget), 'itemid' => $this->modelid, 'contextid' => \context_system::instance()->id, - 'filepath' => '/analysable/' . $this->analysableid . '/' . self::convert_to_int($this->timesplittingid) . '/', + 'filepath' => '/analysable/' . $this->analysableid . '/' . self::clean_time_splitting_id($this->timesplittingid) . '/', 'filename' => self::get_filename($this->evaluation) ]; @@ -127,6 +130,10 @@ class dataset_manager { // Write all this stuff to a tmp file. $filepath = make_request_directory() . DIRECTORY_SEPARATOR . $filerecord['filename']; $fh = fopen($filepath, 'w+'); + if (!$fh) { + $this->close_process(); + throw new \moodle_exception('errorcannotwritedataset', 'analytics', '', $tmpfilepath); + } foreach ($data as $line) { fputcsv($fh, $line); } @@ -144,10 +151,6 @@ class dataset_manager { $this->lock->release(); } - public function release_lock(\core\lock\lock $lock) { - $lock->release(); - } - /** * Returns the previous evaluation file. * @@ -162,7 +165,7 @@ class dataset_manager { $fs = get_file_storage(); // Evaluation data is always labelled. return $fs->get_file(\context_system::instance()->id, 'analytics', self::LABELLED_FILEAREA, $modelid, - '/timesplitting/' . self::convert_to_int($timesplittingid) . '/', self::EVALUATION_FILENAME); + '/timesplitting/' . self::clean_time_splitting_id($timesplittingid) . '/', self::EVALUATION_FILENAME); } public static function delete_previous_evaluation_file($modelid, $timesplittingid) { @@ -183,7 +186,7 @@ class dataset_manager { // Always evaluation.csv and labelled as it is an evaluation file. $filearea = self::get_filearea(true); $filename = self::get_filename(true); - $filepath = '/analysable/' . $analysableid . '/' . self::convert_to_int($timesplittingid) . '/'; + $filepath = '/analysable/' . $analysableid . '/' . self::clean_time_splitting_id($timesplittingid) . '/'; return $fs->get_file(\context_system::instance()->id, 'analytics', $filearea, $modelid, $filepath, $filename); } @@ -235,6 +238,9 @@ class dataset_manager { // Start writing to the merge file. $wh = fopen($tmpfilepath, 'w'); + if (!$wh) { + throw new \moodle_exception('errorcannotwritedataset', 'analytics', '', $tmpfilepath); + } fputcsv($wh, $varnames); fputcsv($wh, $values); @@ -262,7 +268,7 @@ class dataset_manager { 'filearea' => self::get_filearea($includetarget), 'itemid' => $modelid, 'contextid' => \context_system::instance()->id, - 'filepath' => '/timesplitting/' . self::convert_to_int($timesplittingid) . '/', + 'filepath' => '/timesplitting/' . self::clean_time_splitting_id($timesplittingid) . '/', 'filename' => self::get_filename($evaluation) ]; @@ -315,17 +321,14 @@ class dataset_manager { } /** - * I know it is not very orthodox... + * Remove all possibly problematic chars from the time splitting method id (id = its full class name). * - * @param string $string - * @return int + * @param string $timesplittingid + * @return string */ - protected static function convert_to_int($string) { - $sum = 0; - for ($i = 0; $i < strlen($string); $i++) { - $sum += ord($string[$i]); - } - return $sum; + protected static function clean_time_splitting_id($timesplittingid) { + $timesplittingid = str_replace('\\', '-', $timesplittingid); + return clean_param($timesplittingid, PARAM_ALPHANUMEXT); } protected static function get_filename($evaluation) { diff --git a/analytics/classes/local/analyser/base.php b/analytics/classes/local/analyser/base.php index 7d832033c84..96ba73208e5 100644 --- a/analytics/classes/local/analyser/base.php +++ b/analytics/classes/local/analyser/base.php @@ -192,11 +192,11 @@ abstract class base { // Target instances scope is per-analysable (it can't be lower as calculations run once per // analysable, not time splitting method nor time range). - $target = forward_static_call(array($this->target, 'instance')); + $target = call_user_func(array($this->target, 'instance')); // We need to check that the analysable is valid for the target even if we don't include targets // as we still need to discard invalid analysables for the target. - $result = $target->is_valid_analysable($analysable, $includetarget); + $result = $target->is_valid_analysable($analysable, $includetarget, true); if ($result !== true) { $a = new \stdClass(); $a->analysableid = $analysable->get_id(); @@ -217,6 +217,7 @@ abstract class base { $previousanalysis = \core_analytics\dataset_manager::get_evaluation_analysable_file($this->modelid, $analysable->get_id(), $timesplitting->get_id()); + // 1 week is a partly random time interval, no need to worry about DST. $boundary = time() - WEEKSECS; if ($previousanalysis && $previousanalysis->get_timecreated() > $boundary) { // Recover the previous analysed file and avoid generating a new one. @@ -344,18 +345,37 @@ abstract class base { $this->options['evaluation'], !empty($target)); // Flag the model + analysable + timesplitting as being analysed (prevent concurrent executions). - $dataset->init_process(); + if (!$dataset->init_process()) { + // If this model + analysable + timesplitting combination is being analysed we skip this process. + $result->status = \core_analytics\model::NO_DATASET; + $result->message = get_string('analysisinprogress', 'analytics'); + return $result; + } + + // Remove samples the target consider invalid. Note that we use $this->target, $target will be false + // during prediction, but we still need to discard samples the target considers invalid. + $this->target->add_sample_data($samplesdata); + $this->target->filter_out_invalid_samples($sampleids, $analysable, $target); + + if (!$sampleids) { + $result->status = \core_analytics\model::NO_DATASET; + $result->message = get_string('novalidsamples', 'analytics'); + $dataset->close_process(); + return $result; + } foreach ($this->indicators as $key => $indicator) { // The analyser attaches the main entities the sample depends on and are provided to the // indicator to calculate the sample. $this->indicators[$key]->add_sample_data($samplesdata); } + // Provide samples to the target instance (different than $this->target) $target is the new instance we get + // for each analysis in progress. if ($target) { - // Also provided to the target. $target->add_sample_data($samplesdata); } + // Here we start the memory intensive process that will last until $data var is // unset (until the method is finished basically). $data = $timesplitting->calculate($sampleids, $this->get_samples_origin(), $this->indicators, $ranges, $target); @@ -363,6 +383,7 @@ abstract class base { if (!$data) { $result->status = \core_analytics\model::ANALYSE_REJECTED_RANGE_PROCESSOR; $result->message = get_string('novaliddata', 'analytics'); + $dataset->close_process(); return $result; } diff --git a/analytics/classes/local/indicator/community_of_inquiry_activity.php b/analytics/classes/local/indicator/community_of_inquiry_activity.php index d1e280bd72b..e79b297ccda 100644 --- a/analytics/classes/local/indicator/community_of_inquiry_activity.php +++ b/analytics/classes/local/indicator/community_of_inquiry_activity.php @@ -128,6 +128,11 @@ abstract class community_of_inquiry_activity extends linear { } protected function any_feedback($action, \cm_info $cm, $contextid, $user) { + + if (!in_array($action, 'submitted', 'replied', 'viewed')) { + throw new \coding_exception('Provided action "' . $action . '" is not valid.'); + } + if (empty($this->activitylogs[$contextid])) { return false; } diff --git a/analytics/classes/local/indicator/user_track_forums.php b/analytics/classes/local/indicator/user_track_forums.php index e77852e9169..07702cd0ece 100644 --- a/analytics/classes/local/indicator/user_track_forums.php +++ b/analytics/classes/local/indicator/user_track_forums.php @@ -44,10 +44,7 @@ class user_track_forums extends binary { } protected function calculate_sample($sampleid, $samplesorigin, $starttime = false, $endtime = false) { - $user = $this->retrieve('user', $sampleid); - - // TODO Return null if forums tracking is the default. return ($user->trackforums) ? self::get_max_value() : self::get_min_value(); } } diff --git a/analytics/classes/local/target/base.php b/analytics/classes/local/target/base.php index 0f6d4b28122..d25f05f0b63 100644 --- a/analytics/classes/local/target/base.php +++ b/analytics/classes/local/target/base.php @@ -45,7 +45,7 @@ abstract class base extends \core_analytics\calculable { /** * Returns the analyser class that should be used along with this target. * - * @return string + * @return string The full class name as a string */ abstract public function get_analyser_class(); @@ -62,20 +62,21 @@ abstract class base extends \core_analytics\calculable { abstract public function is_valid_analysable(\core_analytics\analysable $analysable, $fortraining = true); /** - * is_valid_sample + * Is this sample from the $analysable valid? * * @param int $sampleid * @param \core_analytics\analysable $analysable - * @return void + * @param bool $fortraining + * @return bool */ - abstract public function is_valid_sample($sampleid, \core_analytics\analysable $analysable); + abstract public function is_valid_sample($sampleid, \core_analytics\analysable $analysable, $fortraining = true); /** * Calculates this target for the provided samples. * * In case there are no values to return or the provided sample is not applicable just return null. * - * @param int $sample + * @param int $sampleid * @param \core_analytics\analysable $analysable * @param int|false $starttime Limit calculations to start time * @param int|false $endtime Limit calculations to end time @@ -103,36 +104,52 @@ abstract class base extends \core_analytics\calculable { return false; } - public function prediction_actions(\core_analytics\prediction $prediction) { - global $PAGE; + /** + * Suggested actions for a user. + * + * @param \core_analytics\prediction $prediction + * @param bool $includedetailsaction + * @return \core_analytics\prediction_action[] + */ + public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false) { + $actions = array(); - $predictionurl = new \moodle_url('/report/insights/prediction.php', - array('id' => $prediction->get_prediction_data()->id)); - if ($predictionurl->compare($PAGE->url)) { - // We don't show the link to prediction.php if we are already in prediction.php - // prediction.php's $PAGE->set_url call is prior to any core_analytics namespace method call. - return array(); + if ($includedetailsaction) { + + $predictionurl = new \moodle_url('/report/insights/prediction.php', + array('id' => $prediction->get_prediction_data()->id)); + + $actions['predictiondetails'] = new \core_analytics\prediction_action('predictiondetails', $prediction, + $predictionurl, new \pix_icon('t/preview', get_string('viewprediction', 'analytics')), + get_string('viewprediction', 'analytics')); } - return array('predictiondetails' => new \core_analytics\prediction_action('predictiondetails', $prediction, $predictionurl, - new \pix_icon('t/preview', get_string('viewprediction', 'analytics')), - get_string('viewprediction', 'analytics')) - ); + return $actions; } /** * Callback to execute once a prediction has been returned from the predictions processor. * + * @param int $modelid * @param int $sampleid + * @param int $rangeindex + * @param \context $samplecontext * @param float|int $prediction * @param float $predictionscore * @return void */ - public function prediction_callback($modelid, $sampleid, $samplecontext, $prediction, $predictionscore) { + public function prediction_callback($modelid, $sampleid, $rangeindex, \context $samplecontext, $prediction, $predictionscore) { return; } - public function generate_insights($modelid, $samplecontexts) { + /** + * Generates insights notifications + * + * @param int $modelid + * @param \context[] $samplecontexts + * @return void + */ + public function generate_insight_notifications($modelid, $samplecontexts) { global $CFG; foreach ($samplecontexts as $context) { @@ -142,12 +159,7 @@ abstract class base extends \core_analytics\calculable { $insightinfo->contextname = $context->get_context_name(); $subject = get_string('insightmessagesubject', 'analytics', $insightinfo); - if ($context->contextlevel >= CONTEXT_COURSE) { - // Course level notification. - $users = get_enrolled_users($context, 'moodle/analytics:listinsights'); - } else { - $users = get_users_by_capability($context, 'moodle/analytics:listinsights'); - } + $users = $this->get_insights_users($context); if (!$coursecontext = $context->get_course_context(false)) { $coursecontext = \context_course::instance(SITEID); @@ -181,6 +193,33 @@ abstract class base extends \core_analytics\calculable { } + /** + * Returns the list of users that will receive insights notifications. + * + * Feel free to overwrite if you need to but keep in mind that moodle/analytics:listinsights + * capability is required to access the list of insights. + * + * @param \context $context + * @return array + */ + protected function get_insights_users(\context $context) { + if ($context->contextlevel >= CONTEXT_COURSE) { + // At course level or below only enrolled users although this is not ideal for + // teachers assigned at category level. + $users = get_enrolled_users($context, 'moodle/analytics:listinsights'); + } else { + $users = get_users_by_capability($context, 'moodle/analytics:listinsights'); + } + return $users; + } + + /** + * Returns an instance of the child class. + * + * Useful to reset cached data. + * + * @return \core_analytics\base\target + */ public static function instance() { return new static(); } @@ -200,7 +239,8 @@ abstract class base extends \core_analytics\calculable { /** * Should the model callback be triggered? * - * @param mixed $class + * @param mixed $predictedvalue + * @param float $predictedscore * @return bool */ public function triggers_callback($predictedvalue, $predictionscore) { @@ -235,11 +275,11 @@ abstract class base extends \core_analytics\calculable { * * @param array $sampleids * @param \core_analytics\analysable $analysable - * @param integer $starttime startime is not necessary when calculating targets - * @param integer $endtime endtime is not necessary when calculating targets + * @param int $starttime + * @param int $endtime * @return array The format to follow is [userid] = scalar|null */ - public function calculate(&$sampleids, \core_analytics\analysable $analysable) { + public function calculate($sampleids, \core_analytics\analysable $analysable, $starttime = false, $endtime = false) { if (!PHPUNIT_TEST && CLI_SCRIPT) { echo '.'; @@ -248,14 +288,8 @@ abstract class base extends \core_analytics\calculable { $calculations = []; foreach ($sampleids as $sampleid => $unusedsampleid) { - if (!$this->is_valid_sample($sampleid, $analysable)) { - // Skip it and remove the sample from the list of calculated samples. - unset($sampleids[$sampleid]); - continue; - } - // No time limits when calculating the target to train models. - $calculatedvalue = $this->calculate_sample($sampleid, $analysable, false, false); + $calculatedvalue = $this->calculate_sample($sampleid, $analysable, $starttime, $endtime); if (!is_null($calculatedvalue)) { if ($this->is_linear() && ($calculatedvalue > static::get_max_value() || $calculatedvalue < static::get_min_value())) { @@ -270,4 +304,20 @@ abstract class base extends \core_analytics\calculable { } return $calculations; } + + /** + * Filters out invalid samples for training. + * + * @param int[] $sampleids + * @param \core_analytics\analysable $analysable + * @return void + */ + public function filter_out_invalid_samples(&$sampleids, \core_analytics\analysable $analysable, $fortraining = true) { + foreach ($sampleids as $sampleid => $unusedsampleid) { + if (!$this->is_valid_sample($sampleid, $analysable, $fortraining)) { + // Skip it and remove the sample from the list of calculated samples. + unset($sampleids[$sampleid]); + } + } + } } diff --git a/analytics/classes/local/time_splitting/base.php b/analytics/classes/local/time_splitting/base.php index b65abec46ee..a828d1466d9 100644 --- a/analytics/classes/local/time_splitting/base.php +++ b/analytics/classes/local/time_splitting/base.php @@ -302,7 +302,7 @@ abstract class base { } protected function get_headers($indicators, $target = false) { - // 3th column will contain the indicator ids. + // 3rd column will contain the indicator ids. $headers = array(); if (!$target) { diff --git a/analytics/classes/local/time_splitting/deciles.php b/analytics/classes/local/time_splitting/deciles.php index d969cc1442b..bf6c3eb9c4c 100644 --- a/analytics/classes/local/time_splitting/deciles.php +++ b/analytics/classes/local/time_splitting/deciles.php @@ -46,6 +46,9 @@ class deciles extends base { for ($i = 0; $i < 10; $i++) { $start = $this->analysable->get_start() + ($rangeduration * $i); $end = $this->analysable->get_start() + ($rangeduration * ($i + 1)); + if ($i === 9) { + $end = $this->analysable->get_end(); + } $ranges[] = array( 'start' => $start, 'end' => $end, diff --git a/analytics/classes/local/time_splitting/deciles_accum.php b/analytics/classes/local/time_splitting/deciles_accum.php index 8eecb1145bf..ff9340fe681 100644 --- a/analytics/classes/local/time_splitting/deciles_accum.php +++ b/analytics/classes/local/time_splitting/deciles_accum.php @@ -45,6 +45,9 @@ class deciles_accum extends base { $ranges = array(); for ($i = 0; $i < 10; $i++) { $end = $this->analysable->get_start() + ($rangeduration * ($i + 1)); + if ($i === 9) { + $end = $this->analysable->get_end(); + } $ranges[] = array( 'start' => $this->analysable->get_start(), 'end' => $end, diff --git a/analytics/classes/local/time_splitting/quarters.php b/analytics/classes/local/time_splitting/quarters.php index c9e55384111..2307dacab23 100644 --- a/analytics/classes/local/time_splitting/quarters.php +++ b/analytics/classes/local/time_splitting/quarters.php @@ -56,8 +56,8 @@ class quarters extends base { 'time' => $this->analysable->get_start() + ($duration * 3) ], [ 'start' => $this->analysable->get_start() + ($duration * 3), - 'end' => $this->analysable->get_start() + ($duration * 4), - 'time' => $this->analysable->get_start() + ($duration * 4) + 'end' => $this->analysable->get_end(), + 'time' => $this->analysable->get_end() ] ]; } diff --git a/analytics/classes/local/time_splitting/quarters_accum.php b/analytics/classes/local/time_splitting/quarters_accum.php index e455341225b..a84d546b95d 100644 --- a/analytics/classes/local/time_splitting/quarters_accum.php +++ b/analytics/classes/local/time_splitting/quarters_accum.php @@ -56,8 +56,8 @@ class quarters_accum extends base { 'time' => $this->analysable->get_start() + ($duration * 3) ], [ 'start' => $this->analysable->get_start(), - 'end' => $this->analysable->get_start() + ($duration * 4), - 'time' => $this->analysable->get_start() + ($duration * 4) + 'end' => $this->analysable->get_end(), + 'time' => $this->analysable->get_end() ] ]; } diff --git a/analytics/classes/manager.php b/analytics/classes/manager.php index 01437837ffd..5b158d4b8d7 100644 --- a/analytics/classes/manager.php +++ b/analytics/classes/manager.php @@ -50,6 +50,27 @@ class manager { */ protected static $alltimesplittings = null; + /** + * Checks that the user can manage models + * + * @throws \required_capability_exception + * @return void + */ + public static function check_can_manage_models() { + require_capability('moodle/analytics:managemodels', \context_system::instance()); + } + + /** + * Checks that the user can list that context insights + * + * @throws \required_capability_exception + * @param \context $context + * @return void + */ + public static function check_can_list_insights(\context $context) { + require_capability('moodle/analytics:listinsights', $context); + } + /** * Returns all system models that match the provided filters. * @@ -61,21 +82,31 @@ class manager { public static function get_all_models($enabled = false, $trained = false, $predictioncontext = false) { global $DB; - $filters = array(); - if ($enabled) { - $filters['enabled'] = 1; + $params = array(); + + $sql = "SELECT DISTINCT am.* FROM {analytics_models} am"; + if ($predictioncontext) { + $sql .= " JOIN {analytics_predictions} ap ON ap.modelid = am.id AND ap.contextid = :contextid"; + $params['contextid'] = $predictioncontext->id; } - if ($trained) { - $filters['trained'] = 1; + + if ($enabled || $trained) { + $conditions = []; + if ($enabled) { + $conditions[] = 'am.enabled = :enabled'; + $params['enabled'] = 1; + } + if ($trained) { + $conditions[] = 'am.trained = :trained'; + $params['trained'] = 1; + } + $sql .= ' WHERE ' . implode(' AND ', $conditions); } - $modelobjs = $DB->get_records('analytics_models', $filters); + $modelobjs = $DB->get_records_sql($sql, $params); $models = array(); foreach ($modelobjs as $modelobj) { - $model = new \core_analytics\model($modelobj); - if (!$predictioncontext || $model->predictions_exist($predictioncontext)) { - $models[$modelobj->id] = $model; - } + $models[$modelobj->id] = new \core_analytics\model($modelobj); } return $models; } @@ -126,6 +157,11 @@ class manager { return self::$predictionprocessors[$checkisready][$predictionclass]; } + /** + * Return all system predictions processors. + * + * @return \core_analytics\predictor + */ public static function get_all_prediction_processors() { $mlbackends = \core_component::get_plugin_list('mlbackend'); @@ -221,6 +257,12 @@ class manager { return self::$allindicators; } + /** + * Returns the specified target + * + * @param mixed $fullclassname + * @return \core_analytics\local\target\base|false False if it is not valid + */ public static function get_target($fullclassname) { if (!self::is_valid($fullclassname, 'core_analytics\local\target\base')) { return false; @@ -245,6 +287,7 @@ class manager { * Returns whether a time splitting method is valid or not. * * @param string $fullclassname + * @param string $baseclass * @return bool */ public static function is_valid($fullclassname, $baseclass) { @@ -257,7 +300,7 @@ class manager { } /** - * get_analytics_logstore + * Returns the logstore used for analytics. * * @return \core\log\sql_reader */ @@ -282,6 +325,56 @@ class manager { return $logstore; } + /** + * Returns the models with insights at the provided context. + * + * @param \context $context + * @return \core_analytics\model[] + */ + public static function get_models_with_insights(\context $context) { + + self::check_can_list_insights($context); + + $models = \core_analytics\manager::get_all_models(true, true, $context); + foreach ($models as $key => $model) { + // Check that it not only have predictions but also generates insights from them. + if (!$model->uses_insights()) { + unset($models[$key]); + } + } + return $models; + } + + /** + * Returns a prediction + * + * @param int $predictionid + * @param bool $requirelogin + * @return array array($model, $prediction, $context) + */ + public static function get_prediction($predictionid, $requirelogin = false) { + global $DB; + + if (!$predictionobj = $DB->get_record('analytics_predictions', array('id' => $predictionid))) { + throw new \moodle_exception('errorpredictionnotfound', 'report_insights'); + } + + if ($requirelogin) { + list($context, $course, $cm) = get_context_info_array($predictionobj->contextid); + require_login($course, false, $cm); + } else { + $context = \context::instance_by_id($predictionobj->contextid); + } + + \core_analytics\manager::check_can_list_insights($context); + + $model = new \core_analytics\model($predictionobj->modelid); + $sampledata = $model->prediction_sample_data($predictionobj); + $prediction = new \core_analytics\prediction($predictionobj, $sampledata); + + return array($model, $prediction, $context); + } + /** * Returns the provided element classes in the site. * @@ -291,7 +384,7 @@ class manager { private static function get_analytics_classes($element) { // Just in case... - $element = clean_param($element, PARAM_ALPHAEXT); + $element = clean_param($element, PARAM_ALPHANUMEXT); $classes = \core_component::get_component_classes_in_namespace('core_analytics', 'local\\' . $element); foreach (\core_component::get_plugin_types() as $type => $unusedplugintypepath) { diff --git a/analytics/classes/model.php b/analytics/classes/model.php index 80a8d6501fb..68386442426 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -42,7 +42,6 @@ class model { const EVALUATE_LOW_SCORE = 4; const EVALUATE_NOT_ENOUGH_DATA = 8; - const ANALYSE_INPROGRESS = 2; const ANALYSE_REJECTED_RANGE_PROCESSOR = 4; const ANALYSABLE_STATUS_INVALID_FOR_RANGEPROCESSORS = 8; const ANALYSABLE_STATUS_INVALID_FOR_TARGET = 16; @@ -88,7 +87,7 @@ class model { global $DB; if (is_scalar($model)) { - $model = $DB->get_record('analytics_models', array('id' => $model)); + $model = $DB->get_record('analytics_models', array('id' => $model), '*', MUST_EXIST); if (!$model) { throw new \moodle_exception('errorunexistingmodel', 'analytics', '', $model); } @@ -266,6 +265,8 @@ class model { public static function create(\core_analytics\local\target\base $target, array $indicators, $timesplittingid = false) { global $USER, $DB; + \core_analytics\manager::check_can_manage_models(); + $indicatorclasses = self::indicator_classes($indicators); $now = time(); @@ -307,6 +308,8 @@ class model { public function update($enabled, $indicators, $timesplittingid = '') { global $USER, $DB; + \core_analytics\manager::check_can_manage_models(); + $now = time(); $indicatorclasses = self::indicator_classes($indicators); @@ -345,6 +348,9 @@ class model { */ public function delete() { global $DB; + + \core_analytics\manager::check_can_manage_models(); + $this->clear_model(); $DB->delete_records('analytics_models', array('id' => $this->model->id)); } @@ -359,6 +365,8 @@ class model { */ public function evaluate($options = array()) { + \core_analytics\manager::check_can_manage_models(); + if ($this->is_static()) { $this->get_analyser()->add_log(get_string('noevaluationbasedassumptions', 'analytics')); $result = new \stdClass(); @@ -366,9 +374,6 @@ class model { return $result; } - // Increase memory limit. - $this->increase_memory(); - $options['evaluation'] = true; $this->init_analyser($options); @@ -376,6 +381,8 @@ class model { throw new \moodle_exception('errornoindicators', 'analytics'); } + $this->heavy_duty_mode(); + // Before get_labelled_data call so we get an early exception if it is not ready. $predictor = \core_analytics\manager::get_predictions_processor(); @@ -438,6 +445,8 @@ class model { public function train() { global $DB; + \core_analytics\manager::check_can_manage_models(); + if ($this->is_static()) { $this->get_analyser()->add_log(get_string('notrainingbasedassumptions', 'analytics')); $result = new \stdClass(); @@ -445,9 +454,6 @@ class model { return $result; } - // Increase memory limit. - $this->increase_memory(); - if (!$this->is_enabled() || empty($this->model->timesplitting)) { throw new \moodle_exception('invalidtimesplitting', 'analytics', '', $this->model->id); } @@ -456,6 +462,8 @@ class model { throw new \moodle_exception('errornoindicators', 'analytics'); } + $this->heavy_duty_mode(); + // Before get_labelled_data call so we get an early exception if it is not writable. $outputdir = $this->get_output_dir(array('execution')); @@ -499,8 +507,7 @@ class model { public function predict() { global $DB; - // Increase memory limit. - $this->increase_memory(); + \core_analytics\manager::check_can_manage_models(); if (!$this->is_enabled() || empty($this->model->timesplitting)) { throw new \moodle_exception('invalidtimesplitting', 'analytics', '', $this->model->id); @@ -510,6 +517,8 @@ class model { throw new \moodle_exception('errornoindicators', 'analytics'); } + $this->heavy_duty_mode(); + // Before get_unlabelled_data call so we get an early exception if it is not writable. $outputdir = $this->get_output_dir(array('execution')); @@ -548,74 +557,19 @@ class model { $result->predictions = $this->get_static_predictions($indicatorcalculations); } else { - // Defer the prediction to the machine learning backend. + // Prediction process runs on the machine learning backend. $predictorresult = $predictor->predict($this->get_unique_id(), $samplesfile, $outputdir); - $result->status = $predictorresult->status; $result->info = $predictorresult->info; - $result->predictions = array(); - if ($predictorresult->predictions) { - foreach ($predictorresult->predictions as $sampleinfo) { - - // We parse each prediction - switch (count($sampleinfo)) { - case 1: - // For whatever reason the predictions processor could not process this sample, we - // skip it and do nothing with it. - debugging($this->model->id . ' model predictions processor could not process the sample with id ' . - $sampleinfo[0], DEBUG_DEVELOPER); - continue; - case 2: - // Prediction processors that do not return a prediction score will have the maximum prediction - // score. - list($uniquesampleid, $prediction) = $sampleinfo; - $predictionscore = 1; - break; - case 3: - list($uniquesampleid, $prediction, $predictionscore) = $sampleinfo; - break; - default: - break; - } - $predictiondata = (object)['prediction' => $prediction, 'predictionscore' => $predictionscore]; - $result->predictions[$uniquesampleid] = $predictiondata; - } - } + $result->predictions = $this->format_predictor_predictions($predictorresult); } - // Here we will store all predictions' contexts, this will be used to limit which users will see those predictions. - $samplecontexts = array(); - if ($result->predictions) { - foreach ($result->predictions as $uniquesampleid => $prediction) { - - if ($this->get_target()->triggers_callback($prediction->prediction, $prediction->predictionscore)) { - - // The unique sample id contains both the sampleid and the rangeindex. - list($sampleid, $rangeindex) = $this->get_time_splitting()->infer_sample_info($uniquesampleid); - - // Store the predicted values. - $samplecontext = $this->save_prediction($sampleid, $rangeindex, $prediction->prediction, $prediction->predictionscore, - json_encode($indicatorcalculations[$uniquesampleid])); - - // Also store all samples context to later generate insights or whatever action the target wants to perform. - $samplecontexts[$samplecontext->id] = $samplecontext; - - $this->get_target()->prediction_callback($this->model->id, $sampleid, $rangeindex, $samplecontext, - $prediction->prediction, $prediction->predictionscore); - } - } + $samplecontexts = $this->execute_prediction_callbacks($result->predictions, $indicatorcalculations); } - if (!empty($samplecontexts)) { - // Notify the target that all predictions have been processed. - $this->get_target()->generate_insights($this->model->id, $samplecontexts); - - // Aggressive invalidation, the cost of filling up the cache is not high. - $cache = \cache::make('core', 'modelswithpredictions'); - foreach ($samplecontexts as $context) { - $cache->delete($context->id); - } + if (!empty($samplecontexts) && $this->uses_insights()) { + $this->trigger_insights($samplecontexts); } $this->flag_file_as_used($samplesfile, 'predicted'); @@ -624,7 +578,108 @@ class model { } /** - * get_static_predictions + * Formats the predictor results. + * + * @param array $predictorresult + * @return array + */ + private function format_predictor_predictions($predictorresult) { + + $predictions = array(); + if ($predictorresult->predictions) { + foreach ($predictorresult->predictions as $sampleinfo) { + + // We parse each prediction + switch (count($sampleinfo)) { + case 1: + // For whatever reason the predictions processor could not process this sample, we + // skip it and do nothing with it. + debugging($this->model->id . ' model predictions processor could not process the sample with id ' . + $sampleinfo[0], DEBUG_DEVELOPER); + continue; + case 2: + // Prediction processors that do not return a prediction score will have the maximum prediction + // score. + list($uniquesampleid, $prediction) = $sampleinfo; + $predictionscore = 1; + break; + case 3: + list($uniquesampleid, $prediction, $predictionscore) = $sampleinfo; + break; + default: + break; + } + $predictiondata = (object)['prediction' => $prediction, 'predictionscore' => $predictionscore]; + $predictions[$uniquesampleid] = $predictiondata; + } + } + return $predictions; + } + + /** + * Execute the prediction callbacks defined by the target. + * + * @param \stdClass[] $predictions + * @param array $predictions + * @return array + */ + protected function execute_prediction_callbacks($predictions, $indicatorcalculations) { + + // Here we will store all predictions' contexts, this will be used to limit which users will see those predictions. + $samplecontexts = array(); + + foreach ($predictions as $uniquesampleid => $prediction) { + + if ($this->get_target()->triggers_callback($prediction->prediction, $prediction->predictionscore)) { + + // The unique sample id contains both the sampleid and the rangeindex. + list($sampleid, $rangeindex) = $this->get_time_splitting()->infer_sample_info($uniquesampleid); + + // Store the predicted values. + $samplecontext = $this->save_prediction($sampleid, $rangeindex, $prediction->prediction, $prediction->predictionscore, + json_encode($indicatorcalculations[$uniquesampleid])); + + // Also store all samples context to later generate insights or whatever action the target wants to perform. + $samplecontexts[$samplecontext->id] = $samplecontext; + + $this->get_target()->prediction_callback($this->model->id, $sampleid, $rangeindex, $samplecontext, + $prediction->prediction, $prediction->predictionscore); + } + } + + return $samplecontexts; + } + + /** + * Generates insights and updates the cache. + * + * @param \context[] $samplecontexts + * @return void + */ + protected function trigger_insights($samplecontexts) { + + // Notify the target that all predictions have been processed. + $this->get_target()->generate_insight_notifications($this->model->id, $samplecontexts); + + // Update cache. + $cache = \cache::make('core', 'contextwithinsights'); + foreach ($samplecontexts as $context) { + $modelids = $cache->get($context->id); + if (!$modelids) { + // The cache is empty, but we don't know if it is empty because there are no insights + // in this context or because cache/s have been purged, we need to be conservative and + // "pay" 1 db read to fill up the cache. + $models = \core_analytics\manager::get_models_with_insights($context); + $cache->set($context->id, array_keys($models)); + } else if (!in_array($this->get_id(), $modelids)) { + array_push($modelids, $this->get_id()); + $cache->set($context->id, $modelids); + } + } + } + + /** + * Get predictions from a static model. * * @param array $indicatorcalculations * @return \stdClass[] @@ -673,8 +728,9 @@ class model { $this->get_target()->add_sample_data($samplesdata); $this->get_target()->add_sample_data($data->indicatorsdata); - // Append new elements (we can not get duplicated because sample-analysable relation is N-1). + // Append new elements (we can not get duplicates because sample-analysable relation is N-1). $range = $this->get_time_splitting()->get_range_by_index($rangeindex); + $this->get_target()->filter_out_invalid_samples($data->sampleids, $data->analysable, false); $calculations = $this->get_target()->calculate($data->sampleids, $data->analysable, $range['start'], $range['end']); // Missing $indicatorcalculations values in $calculations are caused by is_valid_sample. We need to remove @@ -683,7 +739,6 @@ class model { $indicatorcalculations = array_filter($indicatorcalculations, function($indicators, $uniquesampleid) use ($calculations) { list($sampleid, $rangeindex) = $this->get_time_splitting()->infer_sample_info($uniquesampleid); if (!isset($calculations[$sampleid])) { - debugging($uniquesampleid . ' discarded by is_valid_sample'); return false; } return true; @@ -695,7 +750,6 @@ class model { // Null means that the target couldn't calculate the sample, we also remove them from $indicatorcalculations. if (is_null($calculations[$sampleid])) { - debugging($uniquesampleid . ' discarded by is_valid_sample'); unset($indicatorcalculations[$uniquesampleid]); continue; } @@ -747,6 +801,8 @@ class model { public function enable($timesplittingid = false) { global $DB; + \core_analytics\manager::check_can_manage_models(); + $now = time(); if ($timesplittingid && $timesplittingid !== $this->model->timesplitting) { @@ -808,6 +864,8 @@ class model { public function mark_as_trained() { global $DB; + \core_analytics\manager::check_can_manage_models(); + $this->model->trained = 1; $DB->update_record('analytics_models', $this->model); } @@ -873,6 +931,8 @@ class model { public function get_predictions(\context $context) { global $DB; + \core_analytics\manager::check_can_list_insights($context); + // Filters out previous predictions keeping only the last time range one. $sql = "SELECT tip.* FROM {analytics_predictions} tip @@ -917,7 +977,7 @@ class model { } /** - * prediction_sample_data + * Returns the sample data of a prediction. * * @param \stdClass $predictionobj * @return array @@ -934,7 +994,7 @@ class model { } /** - * prediction_sample_description + * Returns the description of a sample * * @param \core_analytics\prediction $prediction * @return array 2 elements: list(string, \renderable) @@ -1004,6 +1064,9 @@ class model { * @return \stdClass */ public function export() { + + \core_analytics\manager::check_can_manage_models(); + $data = clone $this->model; $data->target = $this->get_target()->get_name(); @@ -1027,6 +1090,9 @@ class model { */ public function get_logs($limitfrom = 0, $limitnum = 0) { global $DB; + + \core_analytics\manager::check_can_manage_models(); + return $DB->get_records('analytics_models_log', array('modelid' => $this->get_id()), 'timecreated DESC', '*', $limitfrom, $limitnum); } @@ -1120,14 +1186,21 @@ class model { $DB->delete_records('analytics_train_samples', array('modelid' => $this->model->id)); $DB->delete_records('analytics_used_files', array('modelid' => $this->model->id)); - $cache = \cache::make('core', 'modelswithpredictions'); + // We don't expect people to clear models regularly and the cost of filling the cache is + // 1 db read per context. + $cache = \cache::make('core', 'contextwithinsights'); $result = $cache->purge(); } - private function increase_memory() { + /** + * Increases system memory and time limits. + * + * @return void + */ + private function heavy_duty_mode() { if (ini_get('memory_limit') != -1) { raise_memory_limit(MEMORY_HUGE); } + \core_php_time_limit::raise(); } - } diff --git a/analytics/tests/fixtures/test_target_shortname.php b/analytics/tests/fixtures/test_target_shortname.php index 9b5712de30e..890e7eb5ece 100644 --- a/analytics/tests/fixtures/test_target_shortname.php +++ b/analytics/tests/fixtures/test_target_shortname.php @@ -36,13 +36,16 @@ class test_target_shortname extends \core_analytics\local\target\binary { return true; } - public function is_valid_sample($sampleid, \core_analytics\analysable $analysable) { + public function is_valid_sample($sampleid, \core_analytics\analysable $analysable, $fortraining = true) { + // We skip not-visible courses during training as a way to emulate the training data / prediction data difference. + // In normal circumstances is_valid_sample will return false when they receive a sample that can not be + // processed. + if (!$fortraining) { + return true; + } $sample = $this->retrieve('course', $sampleid); if ($sample->visible == 0) { - // We skip not-visible courses as a way to emulate the training data / prediction data difference. - // In normal circumstances is_valid_sample will return false when they receive a sample that can not be - // processed. return false; } return true; diff --git a/analytics/tests/model_test.php b/analytics/tests/model_test.php index decb11a3509..36c74696ed9 100644 --- a/analytics/tests/model_test.php +++ b/analytics/tests/model_test.php @@ -40,6 +40,8 @@ class analytics_model_testcase extends advanced_testcase { public function setUp() { + $this->setAdminUser(); + $target = \core_analytics\manager::get_target('test_target_shortname'); $indicators = array('test_indicator_max', 'test_indicator_min', 'test_indicator_fullname'); foreach ($indicators as $key => $indicator) { diff --git a/lang/en/analytics.php b/lang/en/analytics.php index 7d737799085..e2bc33dd8be 100644 --- a/lang/en/analytics.php +++ b/lang/en/analytics.php @@ -24,12 +24,16 @@ $string['analysablenotused'] = 'Analysable {$a->analysableid} not used: {$a->errors}'; $string['analysablenotvalidfortarget'] = 'Analysable {$a->analysableid} is not valid for this target: {$a->result}'; +$string['analysisinprogress'] = 'Still being analysed by a previous execution'; $string['analyticslogstore'] = 'Log store used for analytics'; $string['analyticslogstore_help'] = 'The log store that will be used by the analytics API to read users\' activity'; $string['analyticssettings'] = 'Analytics settings'; +$string['coursetoolong'] = 'The course is too long'; $string['enabledtimesplittings'] = 'Time splitting methods'; $string['enabledtimesplittings_help'] = 'The time splitting method divides the course duration in parts, the predictions engine will run at the end of these parts. It is recommended that you only enable the time splitting methods you could be interested on using; the evaluation process will iterate through all of them so the more time splitting methods to go through the slower the evaluation process will be.'; $string['erroralreadypredict'] = '{$a} file has already been used to predict'; +$string['errorcannotreaddataset'] = 'Dataset file {$a} can not be read'; +$string['errorcannotwritedataset'] = 'Dataset file {$a} can not be written'; $string['errorendbeforestart'] = 'The guessed end date ({$a}) is before the course start date.'; $string['errorinvalidindicator'] = 'Invalid {$a} indicator'; $string['errorinvalidtimesplitting'] = 'Invalid time splitting, please ensure you added the class fully qualified class name'; @@ -46,7 +50,7 @@ $string['errorsamplenotavailable'] = 'The predicted sample is not available anym $string['errorunexistingtimesplitting'] = 'The selected time splitting method is not available'; $string['errorunexistingmodel'] = 'Unexisting model {$a}'; $string['errorunknownaction'] = 'Unknown action'; -$string['eventactionclicked'] = 'Prediction action clicked'; +$string['eventpredictionactionstarted'] = 'Prediction action started'; $string['indicator:accessesafterend'] = 'Accesses after the end date'; $string['indicator:accessesbeforestart'] = 'Accesses before the start date'; $string['indicator:anywrite'] = 'Any write action'; @@ -71,6 +75,7 @@ $string['nonewtimeranges'] = 'No new time ranges, nothing to predict'; $string['nopredictionsyet'] = 'No predictions available yet'; $string['notrainingbasedassumptions'] = 'Models based on assumptions do not need training'; $string['novaliddata'] = 'No valid data available'; +$string['novalidsamples'] = 'No valid samples available'; $string['predictionsprocessor'] = 'Predictions processor'; $string['predictionsprocessor_help'] = 'Prediction processors are the machine learning backends that process the datasets generated by calculating models\' indicators and targets.'; $string['processingsitecontents'] = 'Processing site contents'; @@ -87,5 +92,4 @@ $string['timesplitting:weekly'] = 'Weekly'; $string['timesplitting:weeklyaccum'] = 'Weekly accumulative'; $string['timesplittingmethod'] = 'Time splitting method'; $string['timesplittingmethod_help'] = 'The time splitting method divides the course duration in parts, the predictions engine will run at the end of these parts. It is recommended that you only enable the time splitting methods you could be interested on using; the evaluation process will iterate through all of them so the more time splitting methods to go through the slower the evaluation process will be.'; -$string['coursetoolong'] = 'The course is too long'; $string['viewprediction'] = 'View prediction details'; diff --git a/lang/en/cache.php b/lang/en/cache.php index a0bc7412314..caa09914ab0 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -57,7 +57,7 @@ $string['cachedef_langmenu'] = 'List of available languages'; $string['cachedef_message_time_last_message_between_users'] = 'Time created for most recent message between users'; $string['cachedef_locking'] = 'Locking'; $string['cachedef_message_processors_enabled'] = "Message processors enabled status"; -$string['cachedef_modelswithpredictions'] = 'Models with available predictions'; +$string['cachedef_contextwithinsights'] = 'Context with insights'; $string['cachedef_navigation_expandcourse'] = 'Navigation expandable courses'; $string['cachedef_observers'] = 'Event observers'; $string['cachedef_plugin_functions'] = 'Plugins available callbacks'; diff --git a/analytics/classes/event/action_clicked.php b/lib/classes/event/prediction_action_started.php similarity index 77% rename from analytics/classes/event/action_clicked.php rename to lib/classes/event/prediction_action_started.php index 58c080f0ea1..2b25e300557 100644 --- a/analytics/classes/event/action_clicked.php +++ b/lib/classes/event/prediction_action_started.php @@ -28,7 +28,7 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -namespace core_analytics\event; +namespace core\event; defined('MOODLE_INTERNAL') || die(); /** @@ -38,15 +38,21 @@ defined('MOODLE_INTERNAL') || die(); * @copyright 2017 David Monllao {@link http://www.davidmonllao.com} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class action_clicked extends \core\event\base { +class prediction_action_started extends \core\event\base { /** * Set basic properties for the event. */ protected function init() { $this->data['objecttable'] = 'analytics_predictions'; - $this->data['crud'] = 'r'; - $this->data['edulevel'] = self::LEVEL_TEACHING; + + // Marked as create because even if the action is something like viewing a course + // they are starting an action from a prediction, which is kind-of creating an outcome. + $this->data['crud'] = 'c'; + // It will depend on the action, we have no idea really but we need to chose one and + // the user is learning from the prediction so LEVEL_PARTICIPATING seems more appropriate + // than LEVEL_OTHER. + $this->data['edulevel'] = self::LEVEL_PARTICIPATING; } /** @@ -55,7 +61,7 @@ class action_clicked extends \core\event\base { * @return string */ public static function get_name() { - return get_string('eventactionclicked', 'analytics'); + return get_string('eventpredictionactionstarted', 'analytics'); } /** @@ -64,7 +70,7 @@ class action_clicked extends \core\event\base { * @return string */ public function get_description() { - return "The user with id '$this->userid' has clicked '{$this->other['actionname']}' action for the prediction with id '".$this->objectid."'."; + return "The user with id '$this->userid' has started '{$this->other['actionname']}' action for the prediction with id '".$this->objectid."'."; } /** diff --git a/lib/db/caches.php b/lib/db/caches.php index 93a1fa17cc9..70d2202b300 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -312,8 +312,8 @@ $definitions = array( ), ), - // Caches analytic models that have already predicted stuff. - 'modelswithpredictions' => array( + // Caches contexts with insights. + 'contextwithinsights' => array( 'mode' => cache_store::MODE_APPLICATION, 'simplekeys' => true, 'simpledata' => true, diff --git a/lib/mlbackend/php/classes/processor.php b/lib/mlbackend/php/classes/processor.php index 0ad106b5a22..891d1d8eedc 100644 --- a/lib/mlbackend/php/classes/processor.php +++ b/lib/mlbackend/php/classes/processor.php @@ -176,7 +176,7 @@ class processor implements \core_analytics\predictor { * * During evaluation we need to shuffle the evaluation dataset samples to detect deviated results, * if the dataset is massive we can not load everything into memory. We know that 2GB is the - * minimum memory limit we should have (\core_analytics\model::increase_memory), if we substract the memory + * minimum memory limit we should have (\core_analytics\model::heavy_duty_mode), if we substract the memory * that we already consumed and the memory that Phpml algorithms will need we should still have at * least 500MB of memory, which should be enough to evaluate a model. In any case this is a robust * solution that will work for all sites but it should minimize memory limit problems. Site admins diff --git a/lib/mlbackend/python/lang/en/mlbackend_python.php b/lib/mlbackend/python/lang/en/mlbackend_python.php index 129d1adc158..1a2889a4b56 100644 --- a/lib/mlbackend/python/lang/en/mlbackend_python.php +++ b/lib/mlbackend/python/lang/en/mlbackend_python.php @@ -1,5 +1,5 @@ required}" and the installed version is "{$a->installed}"'; -$string['pluginname'] = 'Python predictor'; +$string['pluginname'] = 'Python machine learning backend'; $string['pythonpackagenotinstalled'] = 'moodleinspire python package is not installed or there is a problem with it. Please execute "{$a}" from command line interface for more info'; diff --git a/report/insights/action.php b/report/insights/action.php index 34913a24f03..f0b07c7585e 100644 --- a/report/insights/action.php +++ b/report/insights/action.php @@ -28,36 +28,18 @@ $predictionid = required_param('predictionid', PARAM_INT); $actionname = required_param('action', PARAM_ALPHANUMEXT); $forwardurl = required_param('forwardurl', PARAM_LOCALURL); -if (!$predictionobj = $DB->get_record('analytics_predictions', array('id' => $predictionid))) { - throw new \moodle_exception('errorpredictionnotfound', 'report_insights'); -} - -$context = context::instance_by_id($predictionobj->contextid); - -if ($context->contextlevel === CONTEXT_MODULE) { - list($course, $cm) = get_module_from_cmid($context->instanceid); - require_login($course, true, $cm); -} else if ($context->contextlevel >= CONTEXT_COURSE) { - $coursecontext = $context->get_course_context(true); - require_login($coursecontext->instanceid); -} else { - require_login(); +list($model, $prediction, $context) = \core_analytics\manager::get_prediction($predictionid, true); +if ($context->contextlevel < CONTEXT_COURSE) { + // Only for higher levels than course. $PAGE->set_context($context); } -require_capability('moodle/analytics:listinsights', $context); - -$params = array('predictionid' => $predictionobj->id, 'action' => $actionname, 'forwardurl' => $forwardurl); +$params = array('predictionid' => $prediction->get_prediction_data()->id, 'action' => $actionname, 'forwardurl' => $forwardurl); $url = new \moodle_url('/report/insights/action.php', $params); - -$model = new \core_analytics\model($predictionobj->modelid); -$sampledata = $model->prediction_sample_data($predictionobj); -$prediction = new \core_analytics\prediction($predictionobj, $sampledata); - $PAGE->set_url($url); // Check that the provided action exists. -$actions = $model->get_target()->prediction_actions($prediction); +$actions = $model->get_target()->prediction_actions($prediction, true); if (!isset($actions[$actionname])) { throw new \moodle_exception('errorunknownaction', 'report_insights'); } @@ -81,6 +63,6 @@ $eventdata = array ( 'objectid' => $predictionid, 'other' => array('actionname' => $actionname) ); -\core_analytics\event\action_clicked::create($eventdata)->trigger(); +\core\event\prediction_action_started::create($eventdata)->trigger(); redirect($forwardurl); diff --git a/report/insights/classes/output/insight.php b/report/insights/classes/output/insight.php index 84d5d2b1212..20ad9203fb0 100644 --- a/report/insights/classes/output/insight.php +++ b/report/insights/classes/output/insight.php @@ -45,9 +45,15 @@ class insight implements \renderable, \templatable { */ protected $prediction; - public function __construct(\core_analytics\prediction $prediction, \core_analytics\model $model) { + /** + * @var bool + */ + protected $includedetailsaction = false; + + public function __construct(\core_analytics\prediction $prediction, \core_analytics\model $model, $includedetailsaction = false) { $this->prediction = $prediction; $this->model = $model; + $this->includedetailsaction = $includedetailsaction; } /** @@ -74,7 +80,7 @@ class insight implements \renderable, \templatable { $data->predictiondisplayvalue = $this->model->get_target()->get_display_value($predictedvalue); $data->predictionstyle = $this->get_calculation_style($this->model->get_target(), $predictedvalue); - $actions = $this->model->get_target()->prediction_actions($this->prediction); + $actions = $this->model->get_target()->prediction_actions($this->prediction, $this->includedetailsaction); if ($actions) { $actionsmenu = new \action_menu(); $actionsmenu->set_menu_trigger(get_string('actions')); @@ -106,7 +112,7 @@ class insight implements \renderable, \templatable { } $obj = new \stdClass(); - $obj->name = forward_static_call(array($calculation->indicator, 'get_name'), $calculation->subtype); + $obj->name = call_user_func(array($calculation->indicator, 'get_name')); $obj->displayvalue = $calculation->indicator->get_display_value($calculation->value, $calculation->subtype); $obj->style = $this->get_calculation_style($calculation->indicator, $calculation->value, $calculation->subtype); diff --git a/report/insights/classes/output/insights_list.php b/report/insights/classes/output/insights_list.php index 3c8a6a665aa..97e311c1960 100644 --- a/report/insights/classes/output/insights_list.php +++ b/report/insights/classes/output/insights_list.php @@ -72,7 +72,7 @@ class insights_list implements \renderable, \templatable { $data->insights = array(); foreach ($predictions as $prediction) { - $insightrenderable = new \report_insights\output\insight($prediction, $this->model); + $insightrenderable = new \report_insights\output\insight($prediction, $this->model, true); $data->insights[] = $insightrenderable->export_for_template($output); } diff --git a/report/insights/insights.php b/report/insights/insights.php index 66303cf467a..bddfd98abc4 100644 --- a/report/insights/insights.php +++ b/report/insights/insights.php @@ -27,20 +27,13 @@ require_once(__DIR__ . '/../../config.php'); $contextid = required_param('contextid', PARAM_INT); $modelid = optional_param('modelid', false, PARAM_INT); -$context = context::instance_by_id($contextid); - -if ($context->contextlevel === CONTEXT_MODULE) { - list($course, $cm) = get_module_from_cmid($context->instanceid); - require_login($course, true, $cm); -} else if ($context->contextlevel >= CONTEXT_COURSE) { - $coursecontext = $context->get_course_context(true); - require_login($coursecontext->instanceid); -} else { - require_login(); +list($context, $course, $cm) = get_context_info_array($contextid); +require_login($course, false, $cm); +if ($context->contextlevel < CONTEXT_COURSE) { + // Only for higher levels than course. $PAGE->set_context($context); } - -require_capability('moodle/analytics:listinsights', $context); +\core_analytics\manager::check_can_list_insights($context); // Get all models that are enabled, trained and have predictions at this context. $othermodels = \core_analytics\manager::get_all_models(true, true, $context); diff --git a/report/insights/lib.php b/report/insights/lib.php index bb789a395a0..c6e3b83ba62 100644 --- a/report/insights/lib.php +++ b/report/insights/lib.php @@ -36,11 +36,11 @@ function report_insights_extend_navigation_course($navigation, $course, $context if (has_capability('moodle/analytics:listinsights', $context)) { - $cache = \cache::make('core', 'modelswithpredictions'); + $cache = \cache::make('core', 'contextwithinsights'); $modelids = $cache->get($context->id); if ($modelids === false) { - // Fill the cache. - $models = \core_analytics\manager::get_all_models(true, true, $context); + // They will be full unless a model has been cleared. + $models = \core_analytics\manager::get_models_with_insights($context); $modelids = array_keys($models); $cache->set($context->id, $modelids); } diff --git a/report/insights/prediction.php b/report/insights/prediction.php index a78ab76101d..7e53d837c6e 100644 --- a/report/insights/prediction.php +++ b/report/insights/prediction.php @@ -26,37 +26,19 @@ require_once(__DIR__ . '/../../config.php'); $predictionid = required_param('id', PARAM_INT); -if (!$predictionobj = $DB->get_record('analytics_predictions', array('id' => $predictionid))) { - throw new \moodle_exception('errorpredictionnotfound', 'report_insights'); -} - -$context = context::instance_by_id($predictionobj->contextid); - -if ($context->contextlevel === CONTEXT_MODULE) { - list($course, $cm) = get_module_from_cmid($context->instanceid); - require_login($course, true, $cm); -} else if ($context->contextlevel >= CONTEXT_COURSE) { - $coursecontext = $context->get_course_context(true); - require_login($coursecontext->instanceid); -} else { - require_login(); +list($model, $prediction, $context) = \core_analytics\manager::get_prediction($predictionid, true); +if ($context->contextlevel < CONTEXT_COURSE) { + // Only for higher levels than course. $PAGE->set_context($context); } -require_capability('moodle/analytics:listinsights', $context); - -$params = array('id' => $predictionobj->id); +$params = array('id' => $prediction->get_prediction_data()->id); $url = new \moodle_url('/report/insights/prediction.php', $params); - $PAGE->set_url($url); $PAGE->set_pagelayout('report'); $renderer = $PAGE->get_renderer('report_insights'); -$model = new \core_analytics\model($predictionobj->modelid); -$sampledata = $model->prediction_sample_data($predictionobj); -$prediction = new \core_analytics\prediction($predictionobj, $sampledata); - $insightinfo = new stdClass(); $insightinfo->contextname = $context->get_context_name(); $insightinfo->insightname = $model->get_target()->get_name(); @@ -78,7 +60,7 @@ $PAGE->set_heading($title); echo $OUTPUT->header(); -$renderable = new \report_insights\output\insight($prediction, $model); +$renderable = new \report_insights\output\insight($prediction, $model, false); echo $renderer->render($renderable); echo $OUTPUT->footer();