FormulasQuestion / moodle-qtype_formulas

Formulas question type for Moodle
17 stars 30 forks source link

Question attempts are not "self-contained" #93

Open PhilippImhof opened 1 year ago

PhilippImhof commented 1 year ago

Description of bug / unexpected behavior

Currently, reviewing a quiz attempt may fail in certain cases where a Formulas Question has been modified between the attempt and the review.

Expected behavior

Changes to a question should not make a finished attempt or an answered question undisplayable.

How to reproduce the issue

  1. Create a simple formulas question with a global variable A=1 and set the answer for part 1 to A.
  2. Create a quiz, add the question and attempt the quiz as a student.
  3. Go to the question bank. Change the global variable definition to B=1 and the model answer for part 1 to B. The question is still in a valid state.
  4. As the student, go to the quiz and review your attempt. You will get the following error:
Exception - Variable 'B' has not been defined. in substitute_vname_by_variables

Debug info:
Error code: generalexceptionmessage
Stack trace ``` line 703 of /question/type/formulas/variables.php: Exception thrown line 1045 of /question/type/formulas/variables.php: call to qtype_formulas\variables->substitute_vname_by_variables() line 799 of /question/type/formulas/question.php: call to qtype_formulas\variables->evaluate_general_expression() line 652 of /question/type/formulas/question.php: call to qtype_formulas_question->get_evaluated_answer() line 143 of /question/type/formulas/renderer.php: call to qtype_formulas_question->grade_responses_individually() line 104 of /question/type/formulas/renderer.php: call to qtype_formulas_renderer->get_part_image_and_class() line 69 of /question/type/formulas/renderer.php: call to qtype_formulas_renderer->part_formulation_and_controls() line 385 of /question/engine/renderer.php: call to qtype_formulas_renderer->formulation_and_controls() line 109 of /question/engine/renderer.php: call to core_question_renderer->formulation() line 113 of /question/behaviour/behaviourbase.php: call to core_question_renderer->question() line 907 of /question/engine/questionattempt.php: call to question_behaviour->render() line 461 of /question/engine/questionusage.php: call to question_attempt->render() line 1765 of /mod/quiz/attemptlib.php: call to question_usage_by_activity->render_question() line 1727 of /mod/quiz/attemptlib.php: call to quiz_attempt->render_question_helper() line 187 of /mod/quiz/renderer.php: call to quiz_attempt->render_question() line 56 of /mod/quiz/renderer.php: call to mod_quiz_renderer->questions() line 262 of /mod/quiz/review.php: call to mod_quiz_renderer->review_page() ```

Checking the database shows that in mdl_question_attempts the (evaluated) model answer is available. The offending variable stems from mdl_qtype_formulas_answers where the model answer is stored as B. When fetching this, we should probably also fetch the corresponding record from mdl_qtype_formulas_options, because it contains the updated variable definition B=1.

Additional comments

We have to be very careful and make sure we change this in a way that will still take into account changes, because otherwise automatic regrading (e.g. when a teacher decides they want to give partial credit for some additional answer they did not expect before) will stop working.

PhilippImhof commented 1 year ago

This is probably related to #91 and #18.

PhilippImhof commented 1 year ago

I found the source of the problem:

https://github.com/FormulasQuestion/moodle-qtype_formulas/blob/74ff90d1abd52dce639c872b04b59f7bd5d90d36/question.php#L145-L151

The function above sets the random and global variables to the value they had when the user started the question attempt. Now this is a difficult situation:

I think there is no real solution to this, because if we were to also reset the local variables, grading variables and model answers to the state they had when the student solved the question, it would not possible to correct an error. (Say, we accidentally entered 3*A as the model answer and changed that to 2*A later, this would be ignored.)

PhilippImhof commented 6 months ago

For documentation purposes

Moodle versions up to 3.11

In Moodle 3.11 and below, questions did not have multiple versions. Changes made to the question's settings (e.g. question text, random variables, global variables) and its parts (e.g. subtext, local variables, grading variables, model answer) would automatically have an effect on existing attempts. However, due to the way the Formulas question was implemented, the definition of the (instantiated) random and global variables was saved with the attempt (the attempt step actually). That was done in order to avoid a new randomisation. As a consequence, when an attempted question was later accessed again, e.g. during the review of an attempt, it used the most recent data for everything except for the random and global variables. This could lead to inconsistent states.

Imagine we set the following:

A student attempts the question. Later, we decide to get some more flexibility and change the question to

Now, if the student reviews their attempt, the new question will be loaded, but apply_attempt_step() will overwrite the random and global variables, so C is not a known variable anymore. Therefore, the model answer becomes invalid, the question cannot be shown. Also, the teacher cannot regrade it.

Moodle versions 4.0 and above

From 4.0 on, questions are saved with their versions, i. e. on every change of a question, that question and its parts are saved with in the database with new IDs. Every attempt contains the question ID and is thus linked to the exact version of the question at the time the student did their attempt.

When a teacher regrades the attempt, the link will be updated and the most recent version (actually: the version set in the quiz, e.g. v1 or "always latest") of the question will be used.

Changes with PR #62

From version 6.0.0. on, Formulas question will not store the definition of global and random variables with the attempt steps anymore. Instead, only the PRNG seed will be saved. This means that the random state can always be restored. However, the question will always be in a consistent state.

Note that this can have undesired side effects. Imagine the following:

The student attempts the question and answers 3. They get 0.00 marks, obviously. Now we change the question as follows:

In Moodle 3.11 and below, all these changes become effective immediately. So if the student reviews their attempt, they will see the new question text and their initially bad answer will become correct. The review screen will show a green tick mark next to the input field. However, the grade is saved with the attempt step, so they still have 0.00 marks.

In Moodle 4.0 and above, the changes will only become effective once the teachers regrades the question. In that case, the updated question with its entire definition will be loaded. The student's answer will now be correct and they will get full marks for their attempt.

It would obviously be unethical to change the question in a sense that would invalidate a student's formerly correct answer. But it is important that questions can be edited while still remaining in a consistent state.