From 3d6f2466d9f45feb54ccd23d4b54d969d288b42b Mon Sep 17 00:00:00 2001 From: James Pratt Date: Fri, 14 Feb 2014 15:40:52 +0700 Subject: [PATCH] MDL-43479 quiz response analysis : suppress break down by variants where there are very many variants --- .../stats_from_steps_walkthrough_test.php | 148 +++++++++++++----- .../statistics/questions/calculated.php | 13 ++ .../statistics/questions/calculator.php | 60 ++++--- .../classes/statistics/responses/analyser.php | 15 +- question/type/questiontypebase.php | 10 ++ question/type/upgrade.txt | 8 + 6 files changed, 185 insertions(+), 69 deletions(-) diff --git a/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php b/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php index d0bd87c7607..57b7dee343a 100644 --- a/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php +++ b/mod/quiz/report/statistics/tests/stats_from_steps_walkthrough_test.php @@ -94,15 +94,75 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth $qcalc = new \core_question\statistics\questions\calculator($questions); $this->assertTimeCurrent($qcalc->get_last_calculated_time($qubaids)); - foreach ($questions as $question) { - $qtypeobj = question_bank::get_qtype($question->qtype, false); - if (!$qtypeobj->can_analyse_responses()) { + $expectedvariantcounts = array(2 => array(1 => 6, + 4 => 4, + 5 => 3, + 6 => 4, + 7 => 2, + 8 => 5, + 10 => 1)); + + foreach ($questions as $slot => $question) { + if (!question_bank::get_qtype($question->qtype, false)->can_analyse_responses()) { continue; } $responesstats = new \core_question\statistics\responses\analyser($question); $this->assertTimeCurrent($responesstats->get_last_analysed_time($qubaids)); + $analysis = $responesstats->load_cached($qubaids); + $variantsnos = $analysis->get_variant_nos(); + if (isset($expectedvariantcounts[$slot])) { + // Compare contents, ignore ordering of array, using canonicalize parameter of assertEquals. + $this->assertEquals(array_keys($expectedvariantcounts[$slot]), $variantsnos, '', 0, 10, true); + } else { + $this->assertEquals(array(1), $variantsnos); + } + $totalspervariantno = array(); + foreach ($variantsnos as $variantno) { + + $subpartids = $analysis->get_subpart_ids($variantno); + foreach ($subpartids as $subpartid) { + if (!isset($totalspervariantno[$subpartid])) { + $totalspervariantno[$subpartid] = array(); + } + $totalspervariantno[$subpartid][$variantno] = 0; + + $subpartanalysis = $analysis->get_analysis_for_subpart($variantno, $subpartid); + $classids = $subpartanalysis->get_response_class_ids(); + foreach ($classids as $classid) { + $classanalysis = $subpartanalysis->get_response_class($classid); + $actualresponsecounts = $classanalysis->data_for_question_response_table('', ''); + foreach ($actualresponsecounts as $actualresponsecount) { + $totalspervariantno[$subpartid][$variantno] += $actualresponsecount->count; + } + } + } + } + // Count all counted responses for each part of question and confirm that counted responses, for most question types + // are the number of attempts at the question for each question part. + if ($slot != 5) { + // Slot 5 holds a multi-choice multiple question. + // Multi-choice multiple is slightly strange. Actual answer counts given for each sub part do not add up to the + // total attempt count. + // This is because each option is counted as a sub part and each option can be off or on in each attempt. Off is + // not counted in response analysis for this question type. + foreach ($totalspervariantno as $totalpervariantno) { + if (isset($expectedvariantcounts[$slot])) { + // If we know how many attempts there are at each variant we can check + // that we have counted the correct amount of responses for each variant. + $this->assertEquals($expectedvariantcounts[$slot], + $totalpervariantno, + "Totals responses do not add up in response analysis for slot {$slot}.", + 0, + 10, + true); + } else { + $this->assertEquals(25, + array_sum($totalpervariantno), + "Totals responses do not add up in response analysis for slot {$slot}."); + } + } + } } - for ($rowno = 0; $rowno < $csvdata['responsecounts']->getRowCount(); $rowno++) { $responsecount = $csvdata['responsecounts']->getRow($rowno); if ($responsecount['randq'] == '') { @@ -114,7 +174,7 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth $this->assert_response_count_equals($question, $qubaids, $responsecount); } - // These quiz stats and the question stats found in qstats00.csv were calculated independently in spreadsheet which is + // These quiz stats and the question stats found in qstats00.csv were calculated independently in spreadsheet which is // available in open document or excel format here : // https://github.com/jamiepratt/moodle-quiz-tools/tree/master/statsspreadsheet $quizstatsexpected = array( @@ -186,6 +246,48 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth $this->assert_stat_equals($questionstats, 2, $variant, null, $statname, $expected); } } + foreach ($expectedvariantcounts as $slot => $expectedvariantcount) { + foreach ($expectedvariantcount as $variantno => $s) { + $this->assertEquals($s, $questionstats->for_slot($slot, $variantno)->s); + } + } + } + + /** + * Check that the stat is as expected within a reasonable tolerance. + * + * @param \core_question\statistics\questions\all_calculated_for_qubaid_condition $questionstats + * @param int $slot + * @param int|null $variant if null then not a variant stat. + * @param string|null $subqname if null then not an item stat. + * @param string $statname + * @param float $expected + */ + protected function assert_stat_equals($questionstats, $slot, $variant, $subqname, $statname, $expected) { + + if ($variant === null && $subqname === null) { + $actual = $questionstats->for_slot($slot)->{$statname}; + } else if ($subqname !== null) { + $actual = $questionstats->for_subq($this->randqids[$slot][$subqname])->{$statname}; + } else { + $actual = $questionstats->for_slot($slot, $variant)->{$statname}; + } + if (is_bool($expected) || is_string($expected)) { + $this->assertEquals($expected, $actual, "$statname for slot $slot"); + } else { + switch ($statname) { + case 'covariance' : + case 'discriminationindex' : + case 'discriminativeefficiency' : + case 'effectiveweight' : + $precision = 1e-5; + break; + default : + $precision = 1e-6; + } + $delta = abs($expected) * $precision; + $this->assertEquals(floatval($expected), $actual, "$statname for slot $slot", $delta); + } } protected function assert_response_count_equals($question, $qubaids, $responsecount) { @@ -242,40 +344,4 @@ class quiz_report_statistics_from_steps_testcase extends mod_quiz_attempt_walkth return array($subpartid, $responseclassid); } - /** - * Check that the stat is as expected within a reasonable tolerance. - * - * @param \core_question\statistics\questions\all_calculated_for_qubaid_condition $questionstats - * @param int $slot - * @param int|null $variant if null then not a variant stat. - * @param string|null $subqname if null then not an item stat. - * @param string $statname - * @param float $expected - */ - protected function assert_stat_equals($questionstats, $slot, $variant, $subqname, $statname, $expected) { - - if ($variant === null && $subqname === null) { - $actual = $questionstats->for_slot($slot)->{$statname}; - } else if ($subqname !== null) { - $actual = $questionstats->for_subq($this->randqids[$slot][$subqname])->{$statname}; - } else { - $actual = $questionstats->for_slot($slot, $variant)->{$statname}; - } - if (is_bool($expected) || is_string($expected)) { - $this->assertEquals($expected, $actual, "$statname for slot $slot"); - } else { - switch ($statname) { - case 'covariance' : - case 'discriminationindex' : - case 'discriminativeefficiency' : - case 'effectiveweight' : - $precision = 1e-5; - break; - default : - $precision = 1e-6; - } - $delta = abs($expected) * $precision; - $this->assertEquals(floatval($expected), $actual, "$statname for slot $slot", $delta); - } - } } diff --git a/question/classes/statistics/questions/calculated.php b/question/classes/statistics/questions/calculated.php index aa0b960a250..c64ab1d986d 100644 --- a/question/classes/statistics/questions/calculated.php +++ b/question/classes/statistics/questions/calculated.php @@ -272,4 +272,17 @@ class calculated { return array(); } } + + public function break_down_by_variant() { + $qtype = \question_bank::get_qtype($this->question->qtype); + return $qtype->break_down_stats_and_response_analysis_by_variant($this->question); + } + + + /** + * Delete the data structure for storing variant stats. + */ + public function clear_variants() { + $this->variantstats = array(); + } } diff --git a/question/classes/statistics/questions/calculator.php b/question/classes/statistics/questions/calculator.php index 107024b60ed..56e30345a3a 100644 --- a/question/classes/statistics/questions/calculator.php +++ b/question/classes/statistics/questions/calculator.php @@ -99,15 +99,17 @@ class calculator { $this->progress->increment_progress(); $israndomquestion = ($step->questionid != $this->stats->for_slot($step->slot)->questionid); + $breakdownvariants = !$israndomquestion && $this->stats->for_slot($step->slot)->break_down_by_variant(); // If this is a variant we have not seen before create a place to store stats calculations for this variant. - if (!$israndomquestion && is_null($this->stats->for_slot($step->slot , $step->variant))) { - $this->stats->initialise_for_slot($step->slot, $this->stats->for_slot($step->slot)->question, $step->variant); + if ($breakdownvariants && is_null($this->stats->for_slot($step->slot , $step->variant))) { + $question = $this->stats->for_slot($step->slot)->question; + $this->stats->initialise_for_slot($step->slot, $question, $step->variant); $this->stats->for_slot($step->slot, $step->variant)->randomguessscore = - $this->get_random_guess_score($this->stats->for_slot($step->slot)->question); + $this->get_random_guess_score($question); } // Step data walker for main question. - $this->initial_steps_walker($step, $this->stats->for_slot($step->slot), $summarks, true, !$israndomquestion); + $this->initial_steps_walker($step, $this->stats->for_slot($step->slot), $summarks, true, $breakdownvariants); // If this is a random question do the calculations for sub question stats. if ($israndomquestion) { @@ -154,14 +156,13 @@ class calculator { $this->stats->for_subq($qid)->question = $subquestion; $this->stats->for_subq($qid)->randomguessscore = $this->get_random_guess_score($subquestion); - $this->stats->for_subq($qid)->sort_variants(); if ($variants = $this->stats->get_variants_for_subq($qid)) { foreach ($variants as $variant) { $this->stats->for_subq($qid, $variant)->question = $subquestion; $this->stats->for_subq($qid, $variant)->randomguessscore = $this->get_random_guess_score($subquestion); } + $this->stats->for_subq($qid)->sort_variants(); } - $this->initial_question_walker($this->stats->for_subq($qid)); if ($this->stats->for_subq($qid)->usedin) { @@ -206,9 +207,9 @@ class calculator { foreach ($lateststeps as $step) { $this->progress->increment_progress(); $israndomquestion = ($this->stats->for_slot($step->slot)->question->qtype == 'random'); - $this->secondary_steps_walker($step, $this->stats->for_slot($step->slot), $summarks, !$israndomquestion); + $this->secondary_steps_walker($step, $this->stats->for_slot($step->slot), $summarks); - if ($this->stats->for_slot($step->slot)->subquestions) { + if ($israndomquestion) { $this->secondary_steps_walker($step, $this->stats->for_subq($step->questionid), $summarks); } } @@ -299,6 +300,10 @@ class calculator { } /** + * Calculating the stats is a four step process. + * + * We loop through all 'last step' data first. + * * Update $stats->totalmarks, $stats->markarray, $stats->totalothermarks * and $stats->othermarksarray to include another state. * @@ -323,18 +328,18 @@ class calculator { } if ($dovariantalso) { $this->initial_steps_walker($step, $stats->variantstats[$step->variant], $summarks, $positionstat, false); - } } /** + * Then loop through all questions for the first time. + * * Perform some computations on the per-question statistics calculations after * we have been through all the step data. * * @param calculated $stats question stats to update. - * @param bool $dovariantsalso do we also want to do the same calculations for the variants? */ - protected function initial_question_walker($stats, $dovariantsalso = true) { + protected function initial_question_walker($stats) { $stats->markaverage = $stats->totalmarks / $stats->s; if ($stats->maxmark != 0) { @@ -350,23 +355,28 @@ class calculator { sort($stats->markarray, SORT_NUMERIC); sort($stats->othermarksarray, SORT_NUMERIC); - if ($dovariantsalso) { - foreach ($stats->variantstats as $variantstat) { - $this->initial_question_walker($variantstat, false); - } + // Here we have collected enough data to make the decision about which questions have variants whose stats we also want to + // calculate. We delete the initialised structures where they are not needed. + if (!$stats->get_variants() || !$stats->break_down_by_variant()) { + $stats->clear_variants(); + } + + foreach ($stats->get_variants() as $variant) { + $this->initial_question_walker($stats->variantstats[$variant]); } } /** + * Loop through all last step data again. + * * Now we know the averages, accumulate the date needed to compute the higher * moments of the question scores. * * @param object $step the state to add to the statistics. * @param calculated $stats the question statistics we are accumulating. * @param array $summarks of the sum of marks for each question usage, indexed by question usage id - * @param bool $dovariantalso do we also want to do the same calculations for the variant? */ - protected function secondary_steps_walker($step, $stats, $summarks, $dovariantalso = true) { + protected function secondary_steps_walker($step, $stats, $summarks) { $markdifference = $step->mark - $stats->markaverage; if ($stats->subquestion) { $othermarkdifference = $summarks[$step->questionusageid] - $stats->othermarkaverage; @@ -384,19 +394,19 @@ class calculator { $stats->covariancemaxsum += $sortedmarkdifference * $sortedothermarkdifference; $stats->covariancewithoverallmarksum += $markdifference * $overallmarkdifference; - if ($dovariantalso) { - $this->secondary_steps_walker($step, $stats->variantstats[$step->variant], $summarks, false); + if (isset($stats->variantstats[$step->variant])) { + $this->secondary_steps_walker($step, $stats->variantstats[$step->variant], $summarks); } } /** + * And finally loop through all the questions again. + * * Perform more per-question statistics calculations. * * @param calculated $stats question stats to update. - * @param bool $dovariantsalso do we also want to do the same calculations for the variants? */ - protected function secondary_question_walker($stats, $dovariantsalso = true) { - + protected function secondary_question_walker($stats) { if ($stats->s > 1) { $stats->markvariance = $stats->markvariancesum / ($stats->s - 1); $stats->othermarkvariance = $stats->othermarkvariancesum / ($stats->s - 1); @@ -435,10 +445,8 @@ class calculator { $stats->discriminativeefficiency = null; } - if ($dovariantsalso) { - foreach ($stats->variantstats as $variantstat) { - $this->secondary_question_walker($variantstat, false); - } + foreach ($stats->variantstats as $variantstat) { + $this->secondary_question_walker($variantstat); } } diff --git a/question/classes/statistics/responses/analyser.php b/question/classes/statistics/responses/analyser.php index 191cf938b20..96fdc4178a8 100644 --- a/question/classes/statistics/responses/analyser.php +++ b/question/classes/statistics/responses/analyser.php @@ -52,6 +52,12 @@ class analyser { */ public $responseclasses = array(); + /** + * @var bool whether to break down response analysis by variant. This only applies to questions that have variants and is + * used to suppress the break down of analysis by variant when there are going to be very many variants. + */ + protected $breakdownbyvariant; + /** * Create a new instance of this class for holding/computing the statistics * for a particular question. @@ -62,7 +68,7 @@ class analyser { $this->questiondata = $questiondata; $qtypeobj = \question_bank::get_qtype($this->questiondata->qtype); $this->analysis = new analysis_for_question($qtypeobj->get_possible_responses($this->questiondata)); - + $this->breakdownbyvariant = $qtypeobj->break_down_stats_and_response_analysis_by_variant($this->questiondata); } /** @@ -119,7 +125,12 @@ class analyser { // Analyse it. foreach ($questionattempts as $qa) { $responseparts = $qa->classify_response(); - $this->analysis->count_response_parts($qa->get_variant(), $responseparts); + if ($this->breakdownbyvariant) { + $this->analysis->count_response_parts($qa->get_variant(), $responseparts); + } else { + $this->analysis->count_response_parts(1, $responseparts); + } + } $this->analysis->cache($qubaids, $this->questiondata->id); return $this->analysis; diff --git a/question/type/questiontypebase.php b/question/type/questiontypebase.php index 6a8ac6c64d7..67fbbe70488 100644 --- a/question/type/questiontypebase.php +++ b/question/type/questiontypebase.php @@ -860,6 +860,16 @@ class question_type { return 0; } + /** + * Whether or not to break down question stats and response analysis, for a question defined by $questiondata. + * + * @param object $questiondata The full question definition data. + * @return bool + */ + public function break_down_stats_and_response_analysis_by_variant($questiondata) { + return true; + } + /** * This method should return all the possible types of response that are * recognised for this question. diff --git a/question/type/upgrade.txt b/question/type/upgrade.txt index ccc2c52fde7..08de8c6ee6d 100644 --- a/question/type/upgrade.txt +++ b/question/type/upgrade.txt @@ -1,5 +1,13 @@ This files describes API changes for question type plugins. +=== 2.7 === + + We have added a new method to the question_type base class 'break_down_stats_and_response_analysis_by_variant'. By default it + returns true. If your question type does not have variants of question instances then you can ignore this method as it only + applies to question types that have variants. If a question type does have variants the default action is to break down + response analysis and question stats by variant. But for some question types there might be an almost infinite quantity of + variants for the question, in this case you can suppress break down by variant by returning false from this method. See for + example the non-core question type varnumeric or the slightly more complex stack question type. + === 2.6 === + The changes in MDL-32750 were reverted in favour of the new pdw toggle toolbars plugin for TinyMCE. The get_non_collapsible_editor_options method has been deprecated. -- 2.17.1