From 3a6eb8efdf6df059adfe6a12213a27b98c0cde9a Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Wed, 29 Jun 2011 15:41:56 +0100 Subject: [PATCH] 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. --- question/type/multianswer/renderer.php | 2 + question/type/numerical/question.php | 50 +++++++++++++---- question/type/numerical/questiontype.php | 13 +++-- question/type/numerical/renderer.php | 11 ++-- .../simpletest/testanswerprocessor.php | 55 ++++++++++--------- .../numerical/simpletest/testquestion.php | 2 +- 6 files changed, 85 insertions(+), 48 deletions(-) diff --git a/question/type/multianswer/renderer.php b/question/type/multianswer/renderer.php index 03d8b3899df..36e1c08299d 100644 --- 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 990d11b5438..3fe3b801837 100644 --- 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 90bd215b7f7..5090b00206a 100644 --- 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 50e30f050c7..cc334c4273b 100644 --- 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 40e988aeb56..f0681c1dbe7 100644 --- 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 5c1cc40349f..090f85d4ea0 100644 --- 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'))); }