From cc9fc649a155be2571daec7eded1a88418f11899 Mon Sep 17 00:00:00 2001 From: Henning Bostelmann Date: Sat, 8 Oct 2011 23:02:52 +0100 Subject: [PATCH] MDL-28219 QE2 adaptive behaviour: fix scores and penalties --- question/behaviour/adaptive/behaviour.php | 73 +++-- question/behaviour/adaptive/renderer.php | 16 +- .../adaptive/simpletest/testwalkthrough.php | 309 +++++++++++++++++- .../simpletest/testwalkthrough.php | 7 +- question/engine/questionattempt.php | 15 + question/type/numerical/question.php | 2 +- 6 files changed, 379 insertions(+), 43 deletions(-) diff --git a/question/behaviour/adaptive/behaviour.php b/question/behaviour/adaptive/behaviour.php index 5bcee3be117..c663b522268 100644 --- a/question/behaviour/adaptive/behaviour.php +++ b/question/behaviour/adaptive/behaviour.php @@ -127,16 +127,24 @@ class qbehaviour_adaptive extends question_behaviour_with_save { return $status; } + $prevstep = $this->qa->get_last_step_with_behaviour_var('_try'); + $prevresponse = $prevstep->get_qt_data(); $prevtries = $this->qa->get_last_behaviour_var('_try', 0); $prevbest = $pendingstep->get_fraction(); if (is_null($prevbest)) { $prevbest = 0; } + if ($this->question->is_same_response($response, $prevresponse)) { + return question_attempt::DISCARD; + } + list($fraction, $state) = $this->question->grade_response($response); $pendingstep->set_fraction(max($prevbest, $this->adjusted_fraction($fraction, $prevtries))); - if ($state == question_state::$gradedright) { + if ($prevstep->get_state() == question_state::$complete) { + $pendingstep->set_state(question_state::$complete); + } else if ($state == question_state::$gradedright) { $pendingstep->set_state(question_state::$complete); } else { $pendingstep->set_state(question_state::$todo); @@ -153,32 +161,59 @@ class qbehaviour_adaptive extends question_behaviour_with_save { return question_attempt::DISCARD; } - $laststep = $this->qa->get_last_step(); - $response = $laststep->get_qt_data(); - if (!$this->question->is_gradable_response($response)) { - $pendingstep->set_state(question_state::$gaveup); - return question_attempt::KEEP; - } - $prevtries = $this->qa->get_last_behaviour_var('_try', 0); - $prevbest = $pendingstep->get_fraction(); + $prevbest = $this->qa->get_fraction(); if (is_null($prevbest)) { $prevbest = 0; } - if ($laststep->has_behaviour_var('_try')) { - // Last answer was graded, we want to regrade it. Otherwise the answer - // has changed, and we are grading a new try. - $prevtries -= 1; + $laststep = $this->qa->get_last_step(); + $response = $laststep->get_qt_data(); + if (!$this->question->is_gradable_response($response)) { + $state = question_state::$gaveup; + $fraction = 0; + } else { + + if ($laststep->has_behaviour_var('_try')) { + // Last answer was graded, we want to regrade it. Otherwise the answer + // has changed, and we are grading a new try. + $prevtries -= 1; + } + + list($fraction, $state) = $this->question->grade_response($response); + + $pendingstep->set_behaviour_var('_try', $prevtries + 1); + $pendingstep->set_behaviour_var('_rawfraction', $fraction); + $pendingstep->set_new_response_summary($this->question->summarise_response($response)); } - list($fraction, $state) = $this->question->grade_response($response); - - $pendingstep->set_fraction(max($prevbest, $this->adjusted_fraction($fraction, $prevtries))); $pendingstep->set_state($state); - $pendingstep->set_behaviour_var('_try', $prevtries + 1); - $pendingstep->set_behaviour_var('_rawfraction', $fraction); - $pendingstep->set_new_response_summary($this->question->summarise_response($response)); + $pendingstep->set_fraction(max($prevbest, $this->adjusted_fraction($fraction, $prevtries))); return question_attempt::KEEP; } + + /** + * Got the most recently graded step. This is mainly intended for use by the + * renderer. + * @return question_attempt_step the most recently graded step. + */ + public function get_graded_step() { + $step = $this->qa->get_last_step_with_behaviour_var('_try'); + if ($step->has_behaviour_var('_try')) { + return $step; + } else { + return null; + } + } + + /** + * Determine whether a question state represents an "improvable" result, + * that is, whether the user can still improve their score. + * + * @param question_state $state the question state. + * @return bool whether the state is improvable + */ + public function is_state_improvable(question_state $state) { + return $state == question_state::$todo; + } } diff --git a/question/behaviour/adaptive/renderer.php b/question/behaviour/adaptive/renderer.php index da1a6329050..f5a984e91d9 100644 --- a/question/behaviour/adaptive/renderer.php +++ b/question/behaviour/adaptive/renderer.php @@ -36,13 +36,6 @@ defined('MOODLE_INTERNAL') || die(); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qbehaviour_adaptive_renderer extends qbehaviour_renderer { - protected function get_graded_step(question_attempt $qa) { - foreach ($qa->get_reverse_step_iterator() as $step) { - if ($step->has_behaviour_var('_try')) { - return $step; - } - } - } public function controls(question_attempt $qa, question_display_options $options) { return $this->submit_button($qa, $options); @@ -51,7 +44,7 @@ class qbehaviour_adaptive_renderer extends qbehaviour_renderer { public function feedback(question_attempt $qa, question_display_options $options) { // Try to find the last graded step. - $gradedstep = $this->get_graded_step($qa); + $gradedstep = $qa->get_behaviour()->get_graded_step($qa); if (is_null($gradedstep) || $qa->get_max_mark() == 0 || $options->marks < question_display_options::MARK_AND_MAX) { return ''; @@ -100,14 +93,13 @@ class qbehaviour_adaptive_renderer extends qbehaviour_renderer { } $output = ''; - // print details of grade adjustment due to penalties + // Print details of grade adjustment due to penalties if ($mark->raw != $mark->cur) { $output .= ' ' . get_string('gradingdetailsadjustment', 'qbehaviour_adaptive', $mark); } - // print info about new penalty - // penalty is relevant only if the answer is not correct and further attempts are possible - if (!$qa->get_state()->is_finished()) { + // Print information about any new penalty, only relevant if the answer can be improved. + if ($qa->get_behaviour()->is_state_improvable($qa->get_state())) { $output .= ' ' . get_string('gradingdetailspenalty', 'qbehaviour_adaptive', format_float($qa->get_question()->penalty, $options->markdp)); } diff --git a/question/behaviour/adaptive/simpletest/testwalkthrough.php b/question/behaviour/adaptive/simpletest/testwalkthrough.php index 26eb6411a71..6a6e27c6b0d 100644 --- a/question/behaviour/adaptive/simpletest/testwalkthrough.php +++ b/question/behaviour/adaptive/simpletest/testwalkthrough.php @@ -39,6 +39,18 @@ require_once(dirname(__FILE__) . '/../../../engine/simpletest/helpers.php'); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_base { + protected function get_contains_penalty_info_expectation($penalty) { + $penaltyinfo = get_string('gradingdetailspenalty', 'qbehaviour_adaptive', + format_float($penalty, $this->displayoptions->markdp)); + return new PatternExpectation('/'.preg_quote($penaltyinfo).'/'); + } + + protected function get_does_not_contain_penalty_info_expectation() { + $penaltyinfo = get_string('gradingdetailspenalty', 'qbehaviour_adaptive', 'XXXXX'); + $penaltypattern = '/'.str_replace('XXXXX', '\\w*', preg_quote($penaltyinfo)).'/'; + return new NoPatternExpectation($penaltypattern); + } + public function test_adaptive_multichoice() { // Create a multiple choice, single response question. @@ -72,7 +84,8 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_mc_radio_expectation($wrongindex, true, true), $this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, true, false), $this->get_contains_mc_radio_expectation(($wrongindex + 2) % 3, true, false), - $this->get_contains_incorrect_expectation()); + $this->get_contains_incorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33)); $this->assertPattern('/B|C/', $this->quba->get_response_summary($this->slot)); @@ -102,9 +115,7 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_mc_radio_expectation(($rightindex + 1) % 3, true, false), $this->get_contains_mc_radio_expectation(($rightindex + 2) % 3, true, false), $this->get_contains_correct_expectation(), - new PatternExpectation('/' . preg_quote( - get_string('gradingdetailspenalty', 'qbehaviour_adaptive', - format_float($mc->penalty, $this->displayoptions->markdp))) . '/')); + $this->get_does_not_contain_penalty_info_expectation()); $this->assertEqual('A', $this->quba->get_response_summary($this->slot)); @@ -133,7 +144,8 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b // Now change the correct answer to the question, and regrade. $mc->answers[13]->fraction = -0.33333333; - $mc->answers[15]->fraction = 1; + $mc->answers[14]->fraction = 1; // We don't know which "wrong" index we chose above! + $mc->answers[15]->fraction = 1; // Therefore, treat answers B and C with the same score. $this->quba->regrade_all_questions(); // Verify. @@ -144,7 +156,7 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_partcorrect_expectation()); $autogradedstep = $this->get_step($this->get_step_count() - 2); - $this->assertWithinMargin($autogradedstep->get_fraction(), 0, 0.0000001); + $this->assertWithinMargin($autogradedstep->get_fraction(), 1, 0.0000001); } public function test_adaptive_multichoice2() { @@ -173,14 +185,16 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->check_current_output( $this->get_contains_mark_summary(2), $this->get_contains_submit_button_expectation(true), - $this->get_contains_correct_expectation()); + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_penalty_info_expectation()); - // Save the same correct answer again. Should no do anything. + // Save the same correct answer again. Should not do anything. $numsteps = $this->get_step_count(); $this->process_submission(array('choice0' => 1, 'choice2' => 1)); // Verify. $this->check_step_count($numsteps); + $this->check_current_mark(2); $this->check_current_state(question_state::$complete); // Finish the attempt. @@ -196,6 +210,229 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_correct_expectation()); } + public function test_adaptive_shortanswer_partially_right() { + + // Create a short answer question + $sa = test_question_maker::make_a_shortanswer_question(); + $this->start_attempt_at_question($sa, 'adaptive'); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation()); + + // Submit a partially correct answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'toad')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(0.8); + $this->check_current_output( + $this->get_contains_mark_summary(0.8), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_partcorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit an incorrect answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'bumblebee')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(0.8); + $this->check_current_output( + $this->get_contains_mark_summary(0.8), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit a correct answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'frog')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(0.8); + $this->check_current_output( + $this->get_contains_mark_summary(0.8), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Verify. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(0.8); + $this->check_current_output( + $this->get_contains_mark_summary(0.8), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + } + + public function test_adaptive_shortanswer_wrong_right_wrong() { + + // Create a short answer question + $sa = test_question_maker::make_a_shortanswer_question(); + $this->start_attempt_at_question($sa, 'adaptive'); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation()); + + // Submit a wrong answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'hippopotamus')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(0); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit the same wrong answer again. Nothing should change. + $this->process_submission(array('-submit' => 1, 'answer' => 'hippopotamus')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(0); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit a correct answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'frog')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit another incorrect answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'bumblebee')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Verify. + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + } + + public function test_adaptive_shortanswer_invalid_after_complete() { + + // Create a short answer question + $sa = test_question_maker::make_a_shortanswer_question(); + $this->start_attempt_at_question($sa, 'adaptive'); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation()); + + // Submit a wrong answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'hippopotamus')); + + // Verify. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(0); + $this->check_current_output( + $this->get_contains_mark_summary(0), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit a correct answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'frog')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit an empty answer. + $this->process_submission(array('-submit' => 1, 'answer' => '')); + + // Verify. + $this->check_current_state(question_state::$invalid); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_contains_validation_error_expectation()); + + // Submit another wrong answer. + $this->process_submission(array('-submit' => 1, 'answer' => 'bumblebee')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Verify. + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0.66666667); + $this->check_current_output( + $this->get_contains_mark_summary(0.67), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + } + public function test_adaptive_shortanswer_try_to_submit_blank() { // Create a short answer question with correct answer true. @@ -220,6 +457,7 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_marked_out_of_summary(), $this->get_contains_submit_button_expectation(true), $this->get_does_not_contain_correctness_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), $this->get_contains_validation_error_expectation()); $this->assertNull($this->quba->get_response_summary($this->slot)); @@ -233,6 +471,7 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_mark_summary(0.8), $this->get_contains_submit_button_expectation(true), $this->get_contains_partcorrect_expectation(), + $this->get_contains_penalty_info_expectation(0.33), $this->get_does_not_contain_validation_error_expectation()); // Now submit blank again. @@ -245,6 +484,60 @@ class qbehaviour_adaptive_walkthrough_test extends qbehaviour_walkthrough_test_b $this->get_contains_mark_summary(0.8), $this->get_contains_submit_button_expectation(true), $this->get_contains_partcorrect_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), $this->get_contains_validation_error_expectation()); } + + public function test_adaptive_numerical() { + + // Create a numerical question + $sa = test_question_maker::make_question('numerical', 'pi'); + $this->start_attempt_at_question($sa, 'adaptive'); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_marked_out_of_summary(), + $this->get_contains_submit_button_expectation(true), + $this->get_does_not_contain_feedback_expectation()); + + // Submit the correct answer. + $this->process_submission(array('-submit' => 1, 'answer' => '3.14')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(1); + $this->check_current_output( + $this->get_contains_mark_summary(1), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_correct_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Submit an incorrect answer. + $this->process_submission(array('-submit' => 1, 'answer' => '-5')); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(1); + $this->check_current_output( + $this->get_contains_mark_summary(1), + $this->get_contains_submit_button_expectation(true), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_penalty_info_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Verify. + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(1); + $this->check_current_output( + $this->get_contains_mark_summary(1), + $this->get_contains_submit_button_expectation(false), + $this->get_contains_incorrect_expectation(), + $this->get_does_not_contain_validation_error_expectation()); + } } diff --git a/question/behaviour/adaptivenopenalty/simpletest/testwalkthrough.php b/question/behaviour/adaptivenopenalty/simpletest/testwalkthrough.php index 0cd5ff7d443..ecc45bdc72e 100644 --- a/question/behaviour/adaptivenopenalty/simpletest/testwalkthrough.php +++ b/question/behaviour/adaptivenopenalty/simpletest/testwalkthrough.php @@ -129,7 +129,8 @@ class qbehaviour_adaptivenopenalty_walkthrough_test extends qbehaviour_walkthrou // Now change the correct answer to the question, and regrade. $mc->answers[13]->fraction = -0.33333333; - $mc->answers[15]->fraction = 1; + $mc->answers[14]->fraction = 1; // We don't know which "wrong" index we chose above! + $mc->answers[15]->fraction = 1; // Therefore, treat answers B and C with the same score. $this->quba->regrade_all_questions(); // Verify. @@ -139,8 +140,8 @@ class qbehaviour_adaptivenopenalty_walkthrough_test extends qbehaviour_walkthrou $this->get_contains_mark_summary(1), $this->get_contains_partcorrect_expectation()); - $autogradedstep = $this->get_step($this->get_step_count() - 2); - $this->assertWithinMargin($autogradedstep->get_fraction(), 0, 0.0000001); + $autogradedstep = $this->get_step($this->get_step_count() - 3); + $this->assertWithinMargin($autogradedstep->get_fraction(), 1, 0.0000001); } public function test_multichoice2() { diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 5f29d378d0a..6d4811e8cec 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -415,6 +415,21 @@ class question_attempt { return new question_attempt_step_read_only(); } + /** + * Get the last step with a particular behaviour variable set. + * @param string $name the name of the variable to get. + * @return question_attempt_step the last step, or a step with no variables + * if there was not a real step. + */ + public function get_last_step_with_behaviour_var($name) { + foreach ($this->get_reverse_step_iterator() as $step) { + if ($step->has_behaviour_var($name)) { + return $step; + } + } + return new question_attempt_step_read_only(); + } + /** * Get the latest value of a particular question type variable. That is, get * the value from the latest step that has it set. Return null if it is not diff --git a/question/type/numerical/question.php b/question/type/numerical/question.php index 0e43a847a85..ea1b026fd1f 100644 --- a/question/type/numerical/question.php +++ b/question/type/numerical/question.php @@ -153,7 +153,7 @@ class qtype_numerical_question extends question_graded_automatically { $prevresponse, $newresponse, 'unit'); } - return false; + return true; } public function get_correct_response() {