Moodle
  1. Moodle
  2. MDL-28444

Calculated questions do not synchronize in 2,1

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.1.2
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide
      • Create an empty quiz
      • Create a calculated question "Q1" with data:
        • Question text: "What is {a} plus {b}?"
          ** Answer text: {a}

          +

          {b}, grade 100%
          ** On the following screen, select "Shared datasets" for both {a} and {b}

          . Select "Synchronized".

        • On the third screen, create 10 dataset entries.
      • Create a calculated question "Q2" with data:
        • Question text: "What is {a} times {b}?"
          ** Answer text: {a}

          *

          {b}, grade 100%
          ** On the following screen, select "Shared datasets" for both {a} and {b}

          . Select "Synchronized".

        • Skip the third screen.
      • Add both questions to the quiz, on the same page.
      • Preview the quiz.

      TO VERIFY: Values of

      {a}

      and

      {b}

      are the same in both question texts displayed. (E.g., it says: "Q1: What is 2 plus 3? Q2: What is 2 times 3?"

      Repeat accordingly for the calculatedmulti question type.

      Show
      Create an empty quiz Create a calculated question "Q1" with data: Question text: "What is {a} plus {b}?" ** Answer text: {a} + {b}, grade 100% ** On the following screen, select "Shared datasets" for both {a} and {b} . Select "Synchronized". On the third screen, create 10 dataset entries. Create a calculated question "Q2" with data: Question text: "What is {a} times {b}?" ** Answer text: {a} * {b}, grade 100% ** On the following screen, select "Shared datasets" for both {a} and {b} . Select "Synchronized". Skip the third screen. Add both questions to the quiz, on the same page. Preview the quiz. TO VERIFY: Values of {a} and {b} are the same in both question texts displayed. (E.g., it says: "Q1: What is 2 plus 3? Q2: What is 2 times 3?" Repeat accordingly for the calculatedmulti question type.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      18179

      Description

      Calculated questions with category datasets do not synchronized in quiz
      see http://moodle.org/mod/forum/discuss.php?d=181745

        Activity

        Hide
        Pierre Pichet added a comment -
        
            public function get_variants_selection_seed() {
                if (!empty($this->synchronised) &&
                        $this->datasetloader->datasets_are_synchronised($question->category)) {
                    return 'category' . $this->category;
                } else {
                    return parent::get_variants_selection_seed();
                }
            }
        }
        

        I think that something has been lost on this return category ...
        In 2,0 has there was no link between questions in a quiz, we use the last attempt->time 100 microseconds to use as the itemnumber (adjusted to the real itemnumber of the question) to use in all questions synchronized in the quiz.
        This was justified by the the maximum item number of 100.

        Has I am limited to an Ipad, I could not fully trace back the attempt process.
        However

         * A {@link question_variant_selection_strategy} that is completely random.
         *
         * @copyright  2011 The Open University
         * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
         */
        class question_variant_random_strategy implements question_variant_selection_strategy {
            public function choose_variant($maxvariants, $seed) {
                return rand(1, $maxvariants);
            }
        

        seems where the maxvariant is calculated...

        I hope this can help you

        Show
        Pierre Pichet added a comment - public function get_variants_selection_seed() { if (!empty($this->synchronised) && $this->datasetloader->datasets_are_synchronised($question->category)) { return 'category' . $this->category; } else { return parent::get_variants_selection_seed(); } } } I think that something has been lost on this return category ... In 2,0 has there was no link between questions in a quiz, we use the last attempt->time 100 microseconds to use as the itemnumber (adjusted to the real itemnumber of the question) to use in all questions synchronized in the quiz. This was justified by the the maximum item number of 100. Has I am limited to an Ipad, I could not fully trace back the attempt process. However * A {@link question_variant_selection_strategy} that is completely random. * * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class question_variant_random_strategy implements question_variant_selection_strategy { public function choose_variant($maxvariants, $seed) { return rand(1, $maxvariants); } seems where the maxvariant is calculated... I hope this can help you
        Hide
        Pierre Pichet added a comment -

        As the question category moving process does not handle correctly category datasets unless you change this recently, the test relating the datasets category to the question category should not be done this way until the question moving has been corrected.

        Show
        Pierre Pichet added a comment - As the question category moving process does not handle correctly category datasets unless you change this recently, the test relating the datasets category to the question category should not be done this way until the question moving has been corrected.
        Hide
        Tim Hunt added a comment -

        The quiz actually uses question_variant_pseudorandom_no_repeats_strategy, which should select the same variant for questions with the same variants_selection_seed.

        So, I think the problem is with the get_variants_selection_seed method, and, as you way, may well be to do with the problems of moving category datasets. More investigation needed.

        Show
        Tim Hunt added a comment - The quiz actually uses question_variant_pseudorandom_no_repeats_strategy, which should select the same variant for questions with the same variants_selection_seed. So, I think the problem is with the get_variants_selection_seed method, and, as you way, may well be to do with the problems of moving category datasets. More investigation needed.
        Hide
        Pierre Pichet added a comment -
         
        /** 
        * Start this question attempt. 
        *
        * You should not call this method directly. Call 
        * {@linkquestion_usage_by_activity::start_question()} instead. 
        * 
        * @param string|question_behaviour $preferredbehaviour the name of the 
        * desired archetypal behaviour, or an actual model instance. 
        * @param int $variant the variant of the question to start. Between 1 and 
        * $this->get_question()->get_num_variants() inclusive. 
        * @param array $submitteddata optional, used when re-starting to keep the same initial state. 
        * @param int $timestamp optional, the timstamp to record for this action. Defaults to now. 
        * @param int $userid optional, the user to attribute this action to. Defaults to the current user. 
        */ 
        public function start($preferredbehaviour, $variant, $submitteddata = array(), $timestamp = null, $userid = null) { 
        // Initialise the behaviour. 
        $this->variant = $variant; 
         

        The $variant should be the same for all synchronized calculated or calculatedmulti in the quiz.
        This can be done as long as all the questions use the same initial data.
        Here I am not sure if this condition is valid for all the various possibilities that allow the new engine in a quiz.
        If the student is allowed to answer more than once time in the given quiz attempt, do the $variant remains constant or not ?
        If not then we cannot synchronized questions across the quiz.

        The problem of moving datasets is only that the question could be synchronized with another similar (same name) variable that is not in question category.

        In all cases it will use the same itemnumber that could not contain the same value.

        This is why in 2,0 there was a warning when prewieing the quiz if the question->category was not identical with the dataset->category.

        But first, could we synchronize in all new engine quiz settings or not?
        id46009

        Show
        Pierre Pichet added a comment - /** * Start this question attempt. * * You should not call this method directly. Call * {@linkquestion_usage_by_activity::start_question()} instead. * * @param string|question_behaviour $preferredbehaviour the name of the * desired archetypal behaviour, or an actual model instance. * @param int $variant the variant of the question to start. Between 1 and * $this->get_question()->get_num_variants() inclusive. * @param array $submitteddata optional, used when re-starting to keep the same initial state. * @param int $timestamp optional, the timstamp to record for this action. Defaults to now. * @param int $userid optional, the user to attribute this action to. Defaults to the current user. */ public function start($preferredbehaviour, $variant, $submitteddata = array(), $timestamp = null, $userid = null) { // Initialise the behaviour. $this->variant = $variant; The $variant should be the same for all synchronized calculated or calculatedmulti in the quiz. This can be done as long as all the questions use the same initial data. Here I am not sure if this condition is valid for all the various possibilities that allow the new engine in a quiz. If the student is allowed to answer more than once time in the given quiz attempt, do the $variant remains constant or not ? If not then we cannot synchronized questions across the quiz. The problem of moving datasets is only that the question could be synchronized with another similar (same name) variable that is not in question category. In all cases it will use the same itemnumber that could not contain the same value. This is why in 2,0 there was a warning when prewieing the quiz if the question->category was not identical with the dataset->category. But first, could we synchronize in all new engine quiz settings or not? id46009
        Hide
        Pierre Pichet added a comment -

        Where do you put in 2,1 the equivalent mechanism to synchronize using the
        attempt->timestart used in 2,0 ?

                // Choose a random dataset
                // maxnumber sould not be breater than 100
                if ($maxnumber > CALCULATEDQUESTIONMAXITEMNUMBER ){
                    $maxnumber = CALCULATEDQUESTIONMAXITEMNUMBER ;
                }
                if ( $synchronize_calculated === false ) {
                    $state->options->datasetitem = rand(1, $maxnumber);
                }else{
                    $state->options->datasetitem = intval( $maxnumber * substr($attempt->timestart,-2) /100 ) ;
                    if ($state->options->datasetitem < 1) {
                        $state->options->datasetitem =1 ;
                    } else if ($state->options->datasetitem > $maxnumber){
                        $state->options->datasetitem = $maxnumber ;
                    }
        
                };
                $state->options->dataset =
                    $this->pick_question_dataset($question,$state->options->datasetitem);
                $virtualqtype = $this->get_virtual_qtype( );
        
        Show
        Pierre Pichet added a comment - Where do you put in 2,1 the equivalent mechanism to synchronize using the attempt->timestart used in 2,0 ? // Choose a random dataset // maxnumber sould not be breater than 100 if ($maxnumber > CALCULATEDQUESTIONMAXITEMNUMBER ){ $maxnumber = CALCULATEDQUESTIONMAXITEMNUMBER ; } if ( $synchronize_calculated === false ) { $state->options->datasetitem = rand(1, $maxnumber); }else{ $state->options->datasetitem = intval( $maxnumber * substr($attempt->timestart,-2) /100 ) ; if ($state->options->datasetitem < 1) { $state->options->datasetitem =1 ; } else if ($state->options->datasetitem > $maxnumber){ $state->options->datasetitem = $maxnumber ; } }; $state->options->dataset = $this->pick_question_dataset($question,$state->options->datasetitem); $virtualqtype = $this->get_virtual_qtype( );
        Hide
        Pierre Pichet added a comment -

        The norepeat strategy is an improvment.
        To implement in synchronization, we will need that in the attempt data we add a non repeatable random number at least for 1-100 that could be used by questions.
        The max number could be defined from a question parameter that the attempt building function could use.

        Show
        Pierre Pichet added a comment - The norepeat strategy is an improvment. To implement in synchronization, we will need that in the attempt data we add a non repeatable random number at least for 1-100 that could be used by questions. The max number could be defined from a question parameter that the attempt building function could use.
        Hide
        Pierre Pichet added a comment -

        If this max is set to the same value in calculated and calculatedmulti, then these questions could be synchronized in a given attempt.

        Show
        Pierre Pichet added a comment - If this max is set to the same value in calculated and calculatedmulti, then these questions could be synchronized in a given attempt.
        Hide
        Orestes Mas added a comment -

        I voted for this bug. For us it's an unfortunate and sad regression from 2.0, because we have already developed some quizzes taking advantage of synchronization: They're lab assignments, in which we give students random electronic component values in one question at the beggining, and all subsequent questions rely on these values being the same along the quiz.

        If this bug does not get resolved before september, we'll have to return to 2.0, so please, please...

        Show
        Orestes Mas added a comment - I voted for this bug. For us it's an unfortunate and sad regression from 2.0, because we have already developed some quizzes taking advantage of synchronization: They're lab assignments, in which we give students random electronic component values in one question at the beggining, and all subsequent questions rely on these values being the same along the quiz. If this bug does not get resolved before september, we'll have to return to 2.0, so please, please...
        Hide
        Pierre Pichet added a comment -

        Orestes,
        I was planing use this synchronization feature for a similar reason i.e following the values of chemical titrations with given reactant concentrations.
        However this a minor feature of all the beautiful code that Tim has added to the question and quiz handling.
        In the best case, this could be solved for september but in any case this will be solved and transparent for 2,0 users.
        Unless you have other problems with 2,0, better to stay with 2,0 as you are using it in a real course.
        At UQAM, we are migrating only some experimental courses to 2,0 for september.

        Show
        Pierre Pichet added a comment - Orestes, I was planing use this synchronization feature for a similar reason i.e following the values of chemical titrations with given reactant concentrations. However this a minor feature of all the beautiful code that Tim has added to the question and quiz handling. In the best case, this could be solved for september but in any case this will be solved and transparent for 2,0 users. Unless you have other problems with 2,0, better to stay with 2,0 as you are using it in a real course. At UQAM, we are migrating only some experimental courses to 2,0 for september.
        Hide
        Henning Bostelmann added a comment -

        This bug is caused by a simple typo. See diff.

        Show
        Henning Bostelmann added a comment - This bug is caused by a simple typo. See diff.
        Hide
        Tim Hunt added a comment -

        Doh! Thank you for working that out.

        Just one thing before I submit this for integration: can we write a unit test to prove it is working how, and ensure it does not break in the future?

        Show
        Tim Hunt added a comment - Doh! Thank you for working that out. Just one thing before I submit this for integration: can we write a unit test to prove it is working how, and ensure it does not break in the future?
        Hide
        Tim Hunt added a comment -

        Answer: yes.

        Show
        Tim Hunt added a comment - Answer: yes.
        Hide
        Pierre Pichet added a comment - - edited

        Effectively the proposed change set the synchro on.
        However at first sigth the way you handle synchro and shared datasets seems to have changed from previous versions.
        Some simple tests shows that the sequence of dataitem number is the same for different quiz with the same user.
        For a given shared variable ( 5 items) use in different quiz I got the sequence 3 4 5 1 2 for the three quiz and another variable give ( 10 items) give me another sequence but the same one in different quiz.
        And effectively if you look at the code for shared datasets there is no random.
        And this real random could not be set unless we define a random value at the quiz level ( the timer was used in 2,0).
        This could explain also that if one question not synchronized used a dataset that is synchronized then it receives the synchronized dataset of the other questions.

        Other comments will follow in the next days...

        Show
        Pierre Pichet added a comment - - edited Effectively the proposed change set the synchro on. However at first sigth the way you handle synchro and shared datasets seems to have changed from previous versions. Some simple tests shows that the sequence of dataitem number is the same for different quiz with the same user. For a given shared variable ( 5 items) use in different quiz I got the sequence 3 4 5 1 2 for the three quiz and another variable give ( 10 items) give me another sequence but the same one in different quiz. And effectively if you look at the code for shared datasets there is no random. And this real random could not be set unless we define a random value at the quiz level ( the timer was used in 2,0). This could explain also that if one question not synchronized used a dataset that is synchronized then it receives the synchronized dataset of the other questions. Other comments will follow in the next days...
        Hide
        Pierre Pichet added a comment -

        Tim,
        Your improvment so the values do not repeat until all possibilities are used is a main feature that should be valid also in synchronization.
        Your code avoid as far as possible new quiz-questions parameters (as grading, display ..) that is specific to a given question type so to maintain easy building of new ones.
        Your process to relate the first item number to the category seems reasonable and then cycling by increasing the dataitem number an acceptable solution.

        Next weeks, hopefully before 2,2, I should be able to set a proposal to moving category datasets

        Show
        Pierre Pichet added a comment - Tim, Your improvment so the values do not repeat until all possibilities are used is a main feature that should be valid also in synchronization. Your code avoid as far as possible new quiz-questions parameters (as grading, display ..) that is specific to a given question type so to maintain easy building of new ones. Your process to relate the first item number to the category seems reasonable and then cycling by increasing the dataitem number an acceptable solution. Next weeks, hopefully before 2,2, I should be able to set a proposal to moving category datasets
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        Rossiani Wijaya added a comment -

        Tested and works great.

        Thanks everyone.

        Test passed.

        Show
        Rossiani Wijaya added a comment - Tested and works great. Thanks everyone. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

          People

          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: