jeff/feature-answer-pool

[jaericson]

@nparlante

Creating pull request for the "answer pool" feature on multiple choice questions.

This allows the author to have many choices to their multiple choice questions (some correct and some incorrect) and specify how many choices to display to the student. If you choose n choices, the student will see 1 correct choice and (n-1) incorrect choices, randomly selected from your list of choices and shuffled so that the order of choices the student sees is random.

To try out, you set up a multiple choice problem with attribute answer-pool set to the number of choices you want. Then, you can provide multiple solution explanations within a <solutionset> or just have one solution explanation for all correct choices by including only one <solution> without a <solutionset> (see example 2 below).

The explanation-id attributes match a <choice> to a corresponding <solution>, which is only necessary when you have multiple <solution> elements. You can add several <multiplechoiceresponse> elements to a problem if you'd like, and if you want a different combination of choices on repeated attempts, be sure in the problem's settings to set "Randomization" to "Always".

Example XML:

<problem>
    <p>What is the correct answer?</p>
    <multiplechoiceresponse answer-pool="4">
      <choicegroup type="MultipleChoice">
        <choice correct="false">wrong-1</choice>
        <choice correct="false">wrong-2</choice>
        <choice correct="true" explanation-id="solution1">correct-1</choice>
        <choice correct="false">wrong-3</choice>
        <choice correct="false">wrong-4</choice>
        <choice correct="true" explanation-id="solution2">correct-2</choice>
      </choicegroup>
    </multiplechoiceresponse>

    <solutionset>
        <solution explanation-id="solution1">
        <div class="detailed-solution">
            <p>Explanation</p>
            <p>This is the 1st solution</p>
            <p>Not much to explain here, sorry!</p>
        </div>
        </solution>

        <solution explanation-id="solution2">
        <div class="detailed-solution">
            <p>Explanation</p>
            <p>This is the 2nd solution</p>
        </div>
        </solution>
    </solutionset>
</problem>

or without a <solutionset>:

<problem>
  <p>What is the correct answer?</p>
  <multiplechoiceresponse answer-pool="4">
    <choicegroup type="MultipleChoice">
      <choice correct="false">wrong-1</choice>
      <choice correct="false">wrong-2</choice>
      <choice correct="true">correct-1</choice>
      <choice correct="false">wrong-3</choice>
      <choice correct="false">wrong-4</choice>
      <choice correct="true">correct-2</choice>
    </choicegroup>
  </multiplechoiceresponse>

  <solution>
  <div class="detailed-solution">
      <p>Explanation</p>
      <p>This is the solution</p>
      <p>Not much to explain here, sorry!</p>
  </div>
  </solution>
</problem>

[nparlante]

For logging, what we want is that the names like choice_0 choice_1 etc. reflect the order as seen by the author. Since this code does shuffling, the displayed order is different from the authored order.

Anyway, I checked the event_info for logging. I clicked the topmost choice which happened to be choice_1 in the authored order, then in capa_module check_problem() we have:

'answers': {u'i4x-Stanford-CS99-problem-89017aeeb1e346f8b80cc4b7b1ea0f51_2_1': u'choice_1'},

So it's doing the right thing. The approach is to do the shuffle after the name= are assigned, so then the nodes are moved around with their proper names already present.

[nparlante]

Ok, I clicked around a bunch of tests with the answer-pool feature, checking, saving, and logging and that all LGTM.

[sarina]

I think you might want to rebase this branch (and your other PR branch) - you shouldn't have commits you haven't authored in your PR. That seems wrong to me.

Please get in touch with me if you need help finding edXers to help review your PR.

[jaericson]

Thanks @sarina

Just rebased this branch so that commits from others aren't showing up anymore. Please checkout this branch anew if you want to test anything on it.

[nparlante]

Think you mean N here

[nparlante]

I squished Jeff's many commits down to one and pushed it back here --force. Hmm, now it shows up as "7 days ago" above. Ok next time I'll grab the date from the latest, not the oldest.

[sarina]

@nparlante that's just a git thing, ain't no thang but a git thang

(it's normal you didn't mess anything up is what I'm trying to say :)

[nparlante]

@sarina heh, thanks. Like there's the right way, the git way, and then there's my way!

[nparlante]

Hey Jeff, as we discussed, here's my suggestions:
-Move the attribute down to choicegroup
-Avoid doing the xtree query twice
-Change the function names now that we change the tree instead of copying

[mulby]

How does num_choices relate to len(choices)? I'm assuming they are equal, but since both are passed into this function it is a bit unclear...

This may be more clearly expressed as:

if len(choices) < 1 or len(correct_choices) < 1:
    # handle invalid input

[mulby]

Oh, I apologize, I now understand the intent of num_choices. So I guess this could be:

if len(choices) == 0 or len(choices) < num_choices:
    # raise an error related to not enough choices

if len(correct_choices) < 1:
    # raise an error related to not having enough correct choices

[mulby]

From an analytics standpoint I have the following concern: Will it be clear from the tracking log entries the subset and ordering of the choices that were presented to the user?

I don't see any explicit effort in this patch to ensure that information is tracked, however, I have not carefully reviewed the related events to see if the appropriate information will be captured simply because of the way you are mutating the tree.

[mulby]

Hi @jaericson - I'm taking over for @rocha on this review. We are mostly concerned with analytics implications, but I made a bunch of general comments yesterday. Feel free to reach out to me directly to discuss.

[singingwolfboy]

What's the status of this pull request? Is it actively being worked on, or can it be closed?

[rocha]

@singingwolfboy it is open.

[sarina]

@singingwolfboy this is a Stanford pull request. IIRC @jaericson is a Stanford student so he may have been busy the last weeks of term and now on break. @nparlante or @jinpa would likely know more but they're all on break until at least Monday.

[nparlante]

I'm going to close this one in favor of an umbrella PR of the shuffle feature and all of Jeff's stuff, staring with this: https://github.com/edx/edx-platform/pull/1995