From 2ccad8f967d2f68dff9fd83fa779efb66e8e0461 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Wed, 25 Sep 2013 01:57:03 +0200
Subject: [PATCH] MDL-41954 Fix the grade for assessment calculation in the
 Workshop module

In a pretty rare case of zero variance of received assessments, the "distance"
of the evaluated assessment from the referential ("best") one was not increased
regardless it's actual value. This led to higher grades for assessments in
certain situations (see the tracker for a particular example).
---
 mod/workshop/eval/best/lib.php            |  4 +-
 mod/workshop/eval/best/tests/lib_test.php | 76 +++++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/mod/workshop/eval/best/lib.php b/mod/workshop/eval/best/lib.php
index 546b0e21cc9..d09431d53ec 100644
--- a/mod/workshop/eval/best/lib.php
+++ b/mod/workshop/eval/best/lib.php
@@ -386,7 +386,9 @@ class workshop_best_evaluation extends workshop_evaluation {
             $n     += $weight;
 
             // variations very close to zero are too sensitive to a small change of data values
-            if ($var > 0.01 and $agrade != $rgrade) {
+            $var = max($var, 0.01);
+
+            if ($agrade != $rgrade) {
                 $absdelta   = abs($agrade - $rgrade);
                 $reldelta   = pow($agrade - $rgrade, 2) / ($settings->comparison * $var);
                 $distance  += $absdelta * $reldelta * $weight;
diff --git a/mod/workshop/eval/best/tests/lib_test.php b/mod/workshop/eval/best/tests/lib_test.php
index 9b31e36288f..b2480c1bc6b 100644
--- a/mod/workshop/eval/best/tests/lib_test.php
+++ b/mod/workshop/eval/best/tests/lib_test.php
@@ -239,6 +239,82 @@ class workshopeval_best_evaluation_testcase extends basic_testcase {
             $this->evaluator->assessments_distance($assessment2, $referential, $diminfo, $settings));
 
     }
+
+    public function test_assessments_distance_zero_variance() {
+        // Fixture set-up: an assessment form of the strategy "Number of errors",
+        // three assertions, same weight.
+        $diminfo = array(
+            1 => (object)array('min' => 0, 'max' => 1, 'weight' => 1),
+            2 => (object)array('min' => 0, 'max' => 1, 'weight' => 1),
+            3 => (object)array('min' => 0, 'max' => 1, 'weight' => 1),
+        );
+
+        // Simulate structure returned by {@link workshop_best_evaluation::prepare_data_from_recordset()}
+        $assessments = array(
+            // The first assessment has weight 0 and the assessment was No, No, No.
+            10 => (object)array(
+                'assessmentid' => 10,
+                'weight' => 0,
+                'reviewerid' => 56,
+                'gradinggrade' => null,
+                'submissionid' => 99,
+                'dimgrades' => array(
+                    1 => 0,
+                    2 => 0,
+                    3 => 0,
+                ),
+            ),
+            // The second assessment has weight 1 and assessments was Yes, Yes, Yes.
+            20 => (object)array(
+                'assessmentid' => 20,
+                'weight' => 1,
+                'reviewerid' => 76,
+                'gradinggrade' => null,
+                'submissionid' => 99,
+                'dimgrades' => array(
+                    1 => 1,
+                    2 => 1,
+                    3 => 1,
+                ),
+            ),
+            // The third assessment has weight 1 and assessments was Yes, Yes, Yes too.
+            30 => (object)array(
+                'assessmentid' => 30,
+                'weight' => 1,
+                'reviewerid' => 97,
+                'gradinggrade' => null,
+                'submissionid' => 99,
+                'dimgrades' => array(
+                    1 => 1,
+                    2 => 1,
+                    3 => 1,
+                ),
+            ),
+        );
+
+        // Process assessments in the same way as in the {@link workshop_best_evaluation::process_assessments()}
+        $assessments = $this->evaluator->normalize_grades($assessments, $diminfo);
+        $average = $this->evaluator->average_assessment($assessments);
+        $variances = $this->evaluator->weighted_variance($assessments);
+        foreach ($variances as $dimid => $variance) {
+            $diminfo[$dimid]->variance = $variance;
+        }
+
+        // Simulate the chosen comparison of assessments "fair" (does not really matter here but we need something).
+        $settings = (object)array('comparison' => 5);
+
+        // Exercise SUT: for every assessment, calculate its distance from the average one.
+        $distances = array();
+        foreach ($assessments as $asid => $assessment) {
+            $distances[$asid] = $this->evaluator->assessments_distance($assessment, $average, $diminfo, $settings);
+        }
+
+        // Validate: the first assessment is far far away from the average one ...
+        $this->assertTrue($distances[10] > 0);
+        // ... while the two others were both picked as the referential ones.
+        $this->assertTrue($distances[20] == 0);
+        $this->assertTrue($distances[30] == 0);
+    }
 }