These coding style issues were spotted while working on the issue. No
real changes here, just the coding style fixes attached to the patchset
for convenience.
Similarly to how the scheduled tasks work, we now automatically check
and make sure that all the models specified in the component's
db/analytics.php file exist during the installation or upgrade of the
component.
The original implementation did not work well for checking that the
given model does NOT exist. If no record was found in the
analytics_models table, the code execution continued and it reached the
moment when indicators were checked. If no indicators were provided, the
call ended up with error 'array_keys() expects parameter 1 to be array,
boolean given' (because indicators were set to false).
The functionality of the \core_analytics\manager::add_builtin_models()
method is to be replaced with automatic update of models provided by the
core moodle component. There is no need to call this method explicitly
any more. Instead, adding new models will be done by updating the
lib/db/analytics.php file and bumping the core version.
When the API had originally been designed, it was assumed that the
presence of the time splitting method implies no requirement for the
machine learning backend and that it is a sign of less performance
demanding models. So it seemed to be safe and useful to have such models
automatically enabled. That assumption did not prove to be valid. Most
of the time is spent calculating indicators that depend on log tables.
We realized it would be useful to be able to specify the default time
splitting method without the need to have such models auto-enabled.
Due to the bug, the no_teaching model might have been marked as
not-trained in the database. Static predictions models (i.e. those using
a target based on assumptions, not facts) are always considered as
trained. If they were marked as not-trained, the prediction scheduled
task would skip them and they would produce no predictions.
Ideally, such a fix should be done for all static models. But there is
no easy way to do it during the upgrade where accessing the analytics
API is not possible. I don't think there are many models out there that
would be affected by this so this seems to be good enough solution for
now (and the future ability to reset models will cover the rest).
Static predictions models (i.e. those using a target based on
assumptions, not facts) are always considered as trained. Clearing them
must not mark them as untrained. Doing so would make them being skipped
by the prediction scheduled task.
