MDL-27418 qtype numerical correct grading of 'right value, wrong unit' reponses.

Probably easiest to explain by example: If the right answer is 1.23 m with alternate
unit 100 cm = 1m then the completely right answers are 1.23 m and 123 cm.
Right answer, wrong unit responses include 1.23, 1.23 frogs and 1.23 cm.
123 m is not considered to have the correct numerical value.

We may well re-consider the way this works in a future version of Moodle.

Unit grading was not implemented before Moodle 2.0. In Moodle 2.0, the unit grading
was more permissive than here, for example 123 m would have been graded correctly in
2.0. However, I'm not sure I follow the rationale for that.

diff --git a/question/type/multianswer/renderer.php b/question/type/multianswer/renderer.php
index 03d8b38..36e1c08 100644 (file)
--- a/question/type/multianswer/renderer.php
+++ b/question/type/multianswer/renderer.php
@@ -168,6 +168,8 @@ class qtype_multianswer_textfield_renderer extends qtype_multianswer_subq_render
         $response = $qa->get_last_qt_var($fieldname);
         if ($subq->qtype->name() == 'shortanswer') {
             $matchinganswer = $subq->get_matching_answer(array('answer' => $response));
+        } else if ($subq->qtype->name() == 'numerical') {
+            $matchinganswer = $subq->get_matching_answer($response, 1);
         } else {
             $matchinganswer = $subq->get_matching_answer($response);
         }

diff --git a/question/type/numerical/question.php b/question/type/numerical/question.php
index 990d11b..3fe3b80 100644 (file)
--- a/question/type/numerical/question.php
+++ b/question/type/numerical/question.php
@@ -177,12 +177,22 @@ class qtype_numerical_question extends question_graded_automatically {
      * Get an answer that contains the feedback and fraction that should be
      * awarded for this resonse.
      * @param number $value the numerical value of a response.
+     * @param number $multiplier for the unit the student gave, if any. When no
+     *      unit was given, or an unrecognised unit was given, $multiplier will be null.
      * @return question_answer the matching answer.
      */
-    public function get_matching_answer($value) {
+    public function get_matching_answer($value, $multiplier) {
+        if (!is_null($multiplier)) {
+            $scaledvalue = $value * $multiplier;
+        } else {
+            $scaledvalue = $value;
+        }
         foreach ($this->answers as $aid => $answer) {
-            if ($answer->within_tolerance($value)) {
-                $answer->id = $aid;
+            if ($answer->within_tolerance($scaledvalue)) {
+                $answer->unitisright = !is_null($multiplier);
+                return $answer;
+            } else if ($answer->within_tolerance($value)) {
+                $answer->unitisright = false;
                 return $answer;
             }
         }
@@ -199,8 +209,14 @@ class qtype_numerical_question extends question_graded_automatically {
         return null;
     }
 
-    public function apply_unit_penalty($fraction, $unit) {
-        if (!empty($unit) && $this->ap->is_known_unit($unit)) {
+    /**
+     * Adjust the fraction based on whether the unit was correct.
+     * @param number $fraction
+     * @param bool $unitisright
+     * @return number
+     */
+    public function apply_unit_penalty($fraction, $unitisright) {
+        if ($unitisright) {
             return $fraction;
         }
 
@@ -218,13 +234,15 @@ class qtype_numerical_question extends question_graded_automatically {
         } else {
             $selectedunit = null;
         }
-        list($value, $unit) = $this->ap->apply_units($response['answer'], $selectedunit);
-        $answer = $this->get_matching_answer($value);
+        list($value, $unit, $multiplier) = $this->ap->apply_units(
+                $response['answer'], $selectedunit);
+        
+        $answer = $this->get_matching_answer($value, $multiplier);
         if (!$answer) {
             return array(0, question_state::$gradedwrong);
         }
 
-        $fraction = $this->apply_unit_penalty($answer->fraction, $unit);
+        $fraction = $this->apply_unit_penalty($answer->fraction, $answer->unitisright);
         return array($fraction, question_state::graded_state_for_fraction($fraction));
     }
 
@@ -238,8 +256,8 @@ class qtype_numerical_question extends question_graded_automatically {
         } else {
             $selectedunit = null;
         }
-        list($value, $unit) = $this->ap->apply_units($response['answer'], $selectedunit);
-        $ans = $this->get_matching_answer($value);
+        list($value, $unit, $multiplier) = $this->ap->apply_units($response['answer'], $selectedunit);
+        $ans = $this->get_matching_answer($value, $multiplier);
         if (!$ans) {
             return array($this->id => question_classified_response::no_response());
         }
@@ -251,14 +269,22 @@ class qtype_numerical_question extends question_graded_automatically {
 
         return array($this->id => new question_classified_response($ans->id,
                 $resp,
-                $this->apply_unit_penalty($ans->fraction, $unit)));
+                $this->apply_unit_penalty($ans->fraction, $ans->unitisright)));
     }
 
     public function check_file_access($qa, $options, $component, $filearea, $args,
             $forcedownload) {
         if ($component == 'question' && $filearea == 'answerfeedback') {
+            $question = $qa->get_question();
             $currentanswer = $qa->get_last_qt_var('answer');
-            $answer = $qa->get_question()->get_matching_answer(array('answer' => $currentanswer));
+            if ($this->has_separate_unit_field()) {
+                $selectedunit = $qa->get_last_qt_var('unit');
+            } else {
+                $selectedunit = null;
+            }
+            list($value, $unit, $multiplier) = $question->ap->apply_units(
+                    $currentanswer, $selectedunit);
+            $answer = $question->get_matching_answer($value, $multiplier);
             $answerid = reset($args); // itemid is answer id.
             return $options->feedback && $answerid == $answer->id;
 

diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php
index 90bd215..5090b00 100644 (file)
--- a/question/type/numerical/questiontype.php
+++ b/question/type/numerical/questiontype.php
@@ -449,7 +449,10 @@ class qtype_numerical extends question_type {
      */
     public function apply_unit($rawresponse, $units, $unitsleft) {
         $ap = $this->make_answer_processor($units, $unitsleft);
-        list($value, $unit) = $ap->apply_units($rawresponse);
+        list($value, $unit, $multiplier) = $ap->apply_units($rawresponse);
+        if (!is_null($multiplier)) {
+            $value *= $multiplier;
+        }
         return $value;
     }
 
@@ -633,7 +636,7 @@ class qtype_numerical_answer_processor {
             $regex = "/^$regex/";
         }
         if (!preg_match($regex, $response, $matches)) {
-            return array(null, null);
+            return array(null, null, null);
         }
 
         $numberstring = $matches[0];
@@ -648,12 +651,12 @@ class qtype_numerical_answer_processor {
         }
 
         if ($unit && $this->is_known_unit($unit)) {
-            $value = $numberstring / $this->units[$unit];
+            $multiplier = 1 / $this->units[$unit];
         } else {
-            $value = $numberstring * 1;
+            $multiplier = null;
         }
 
-        return array($value, $unit);
+        return array($numberstring + 0, $unit, $multiplier); // + 0 to convert to number.
     }
 
     /**

diff --git a/question/type/numerical/renderer.php b/question/type/numerical/renderer.php
index 50e30f0..cc334c4 100644 (file)
--- a/question/type/numerical/renderer.php
+++ b/question/type/numerical/renderer.php
@@ -57,10 +57,11 @@ class qtype_numerical_renderer extends qtype_renderer {
 
         $feedbackimg = '';
         if ($options->correctness) {
-            list($value, $unit) = $question->ap->apply_units($currentanswer, $selectedunit);
-            $answer = $question->get_matching_answer($value);
+            list($value, $unit, $multiplier) = $question->ap->apply_units(
+                    $currentanswer, $selectedunit);
+            $answer = $question->get_matching_answer($value, $multiplier);
             if ($answer) {
-                $fraction = $question->apply_unit_penalty($answer->fraction, $unit);
+                $fraction = $question->apply_unit_penalty($answer->fraction, $answer->unitisright);
             } else {
                 $fraction = 0;
             }
@@ -140,9 +141,9 @@ class qtype_numerical_renderer extends qtype_renderer {
         } else {
             $selectedunit = null;
         }
-        list($value, $unit) = $question->ap->apply_units(
+        list($value, $unit, $multiplier) = $question->ap->apply_units(
                 $qa->get_last_qt_var('answer'), $selectedunit);
-        $answer = $question->get_matching_answer($value);
+        $answer = $question->get_matching_answer($value, $multiplier);
 
         if ($answer && $answer->feedback) {
             $feedback = $question->format_text($answer->feedback, $answer->feedbackformat,

diff --git a/question/type/numerical/simpletest/testanswerprocessor.php b/question/type/numerical/simpletest/testanswerprocessor.php
index 40e988a..f0681c1 100644 (file)
--- a/question/type/numerical/simpletest/testanswerprocessor.php
+++ b/question/type/numerical/simpletest/testanswerprocessor.php
@@ -82,15 +82,20 @@ class qtype_numerical_answer_processor_test extends UnitTestCase {
         $this->assertEqual(array(null, null, null, null), $ap->parse_response(','));
     }
 
-    protected function verify_value_and_unit($exectedval, $expectedunit,
+    protected function verify_value_and_unit($exectedval, $expectedunit, $expectedmultiplier,
             qtype_numerical_answer_processor $ap, $input, $separateunit = null) {
-        list($val, $unit) = $ap->apply_units($input, $separateunit);
+        list($val, $unit, $multiplier) = $ap->apply_units($input, $separateunit);
         if (is_null($exectedval)) {
             $this->assertNull($val);
         } else {
             $this->assertWithinMargin($exectedval, $val, 0.0001);
         }
         $this->assertEqual($expectedunit, $unit);
+        if (is_null($expectedmultiplier)) {
+            $this->assertNull($multiplier);
+        } else {
+            $this->assertWithinMargin($expectedmultiplier, $multiplier, 0.0001);
+        }
     }
 
     public function test_apply_units() {
@@ -98,13 +103,13 @@ class qtype_numerical_answer_processor_test extends UnitTestCase {
                 array('m/s' => 1, 'c' => 3.3356409519815E-9,
                         'mph' => 2.2369362920544), false, '.', ',');
 
-        $this->verify_value_and_unit(3e8, 'm/s', $ap, '3x10^8 m/s');
-        $this->verify_value_and_unit(3e8, '', $ap, '3x10^8');
-        $this->verify_value_and_unit(299792458, 'c', $ap, '1c');
-        $this->verify_value_and_unit(0.44704, 'mph', $ap, '0001.000 mph');
+        $this->verify_value_and_unit(3e8, 'm/s', 1, $ap, '3x10^8 m/s');
+        $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8');
+        $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1c');
+        $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000 mph');
 
-        $this->verify_value_and_unit(1, 'frogs', $ap, '1 frogs');
-        $this->verify_value_and_unit(null, null, $ap, '. m/s');
+        $this->verify_value_and_unit(1, 'frogs', null, $ap, '1 frogs');
+        $this->verify_value_and_unit(null, null, null, $ap, '. m/s');
     }
 
     public function test_apply_units_separate_unit() {
@@ -112,39 +117,39 @@ class qtype_numerical_answer_processor_test extends UnitTestCase {
                 array('m/s' => 1, 'c' => 3.3356409519815E-9,
                         'mph' => 2.2369362920544), false, '.', ',');
 
-        $this->verify_value_and_unit(3e8, 'm/s', $ap, '3x10^8', 'm/s');
-        $this->verify_value_and_unit(3e8, '', $ap, '3x10^8', '');
-        $this->verify_value_and_unit(299792458, 'c', $ap, '1', 'c');
-        $this->verify_value_and_unit(0.44704, 'mph', $ap, '0001.000', 'mph');
+        $this->verify_value_and_unit(3e8, 'm/s', 1, $ap, '3x10^8', 'm/s');
+        $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8', '');
+        $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1', 'c');
+        $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000', 'mph');
 
-        $this->verify_value_and_unit(1, 'frogs', $ap, '1', 'frogs');
-        $this->verify_value_and_unit(null, null, $ap, '.', 'm/s');
+        $this->verify_value_and_unit(1, 'frogs', null, $ap, '1', 'frogs');
+        $this->verify_value_and_unit(null, null, null, $ap, '.', 'm/s');
     }
 
     public function test_euro_style() {
         $ap = new qtype_numerical_answer_processor(array(), false, ',', ' ');
 
-        $this->assertEqual(array(-1000, ''), $ap->apply_units('-1 000'));
-        $this->assertEqual(array(3.14159, ''), $ap->apply_units('3,14159'));
+        $this->assertEqual(array(-1000, '', null), $ap->apply_units('-1 000'));
+        $this->assertEqual(array(3.14159, '', null), $ap->apply_units('3,14159'));
     }
 
     public function test_percent() {
         $ap = new qtype_numerical_answer_processor(array('%' => 100), false, '.', ',');
 
-        $this->assertEqual(array('0.03', '%'), $ap->apply_units('3%'));
-        $this->assertEqual(array('1e-8', '%'), $ap->apply_units('1e-6 %'));
-        $this->assertEqual(array('100', ''), $ap->apply_units('100'));
+        $this->assertEqual(array('3', '%', 0.01), $ap->apply_units('3%'));
+        $this->assertEqual(array('1e-6', '%', 0.01), $ap->apply_units('1e-6 %'));
+        $this->assertEqual(array('100', '', null), $ap->apply_units('100'));
     }
 
 
     public function test_currency() {
         $ap = new qtype_numerical_answer_processor(array('$' => 1, '£' => 1), true, '.', ',');
 
-        $this->assertEqual(array('1234.56', '£'), $ap->apply_units('£1,234.56'));
-        $this->assertEqual(array('100', '$'), $ap->apply_units('$100'));
-        $this->assertEqual(array('100', '$'), $ap->apply_units('$100.'));
-        $this->assertEqual(array('100.00', '$'), $ap->apply_units('$100.00'));
-        $this->assertEqual(array('100', ''), $ap->apply_units('100'));
-        $this->assertEqual(array('100', 'frog'), $ap->apply_units('frog 100'));
+        $this->assertEqual(array('1234.56', '£', 1), $ap->apply_units('£1,234.56'));
+        $this->assertEqual(array('100', '$', 1), $ap->apply_units('$100'));
+        $this->assertEqual(array('100', '$', 1), $ap->apply_units('$100.'));
+        $this->assertEqual(array('100.00', '$', 1), $ap->apply_units('$100.00'));
+        $this->assertEqual(array('100', '', null), $ap->apply_units('100'));
+        $this->assertEqual(array('100', 'frog', null), $ap->apply_units('frog 100'));
     }
 }

diff --git a/question/type/numerical/simpletest/testquestion.php b/question/type/numerical/simpletest/testquestion.php
index 5c1cc40..090f85d 100644 (file)
--- a/question/type/numerical/simpletest/testquestion.php
+++ b/question/type/numerical/simpletest/testquestion.php
@@ -98,7 +98,7 @@ class qtype_numerical_question_test extends UnitTestCase {
                 $question->grade_response(array('answer' => '314000000x10^-8m')));
         $this->assertEqual(array(0.8, question_state::$gradedpartial),
                 $question->grade_response(array('answer' => '3.14 cm')));
-        $this->assertEqual(array(0.8, question_state::$gradedpartial),
+        $this->assertEqual(array(0, question_state::$gradedwrong),
                 $question->grade_response(array('answer' => '314 m')));
     }