Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

Multiple question with a single custom item without answer #161546

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Multiple question with a single custom item without answer #161546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d464d2
Made the graphing-inequalities-2 point choice default to no selection.
wrwrwr Nov 8, 2015
bbed991
Made one-item multiple-choice questions respect emptiness of the item.
wrwrwr Nov 14, 2015
266870b
Forward the first message rather than the last (but not an empty stri…
wrwrwr Nov 15, 2015
face5a4
Normalize message for unanswered custom problems.
wrwrwr Nov 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions exercises/graphing_inequalities_2.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@
<ul>
<li>
<label>
<input id="yes" checked name="isSolution" type="radio">
<input id="yes" name="isSolution" type="radio">
<span>Yes</span>
</label>
</li>
Expand All @@ -206,7 +206,7 @@
$("input[name='isSolution']:checked").attr("id")]
</div>
<div class="validator-function">
if (_.isEqual(guess, [[-5, 5], [5, 5], false, true])) {
if (_.isEqual(guess.slice(0, 4), [[-5, 5], [5, 5], false, true]) || guess[4] === null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this have ever fired without that slice there?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case the guess has 5 elements, so no. A simpler fix is to add null as the fifth element to the array, but this is still not how I'd see it.

Logically, this exercise consists of two parts, a graph and a radio selection. It would be nice to reflect this in its structure. I imagine it could look something like this:

<div class="solution" data-type="multiple">
    <div class="sol" data-type="custom">...</div>
    <div class="sol" data-type="radio">...</div>
    <ul class="choices" data-category="true">...</ul>
</div>

This is what drove the changes on the other branch (including making the problem unanswered when no part or just one part is answered). At least the two changes related to radio-in-multiple (the third commit) are needed to make it work.

return "";
}

Expand Down
49 changes: 49 additions & 0 deletions test/answer-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,32 @@
start();
});

asyncTest("multiple with a single custom item", 9, function() {
setupSolutionArea();
var $problem = jQuery("#qunit-fixture .problem").append(
"<div class='solution' data-type='multiple'>" +
"<div class='instruction'>" +
"<input type='text' id='custom-input'>" +
"</div>" +
"<div class='sol' data-type='custom'>" +
"<div class='guess'>[$('#custom-input').val()]</div>" +
"<div class='validator-function'>" +
"return guess[0] == 'empty' ? '' : guess[0] == 5;" +
"</div>" +
"</div>" +
"</div>"
);

var answerData = Khan.answerTypes.multiple.setup($("#solutionarea"),
$problem.children(".solution"));

testMultipleAnswer(answerData, ["5"], "right", "right answer is right");
testMultipleAnswer(answerData, ["empty"], "empty", "empty answer is empty");
testMultipleAnswer(answerData, ["1"], "wrong", "wrong answer is wrong");

start();
});

asyncTest("set with no things", 15, function() {
setupSolutionArea();
var $problem = jQuery("#qunit-fixture .problem").append(
Expand Down Expand Up @@ -985,6 +1011,29 @@
start();
});

asyncTest("custom", 9, function() {
setupSolutionArea();
var $problem = jQuery("#qunit-fixture .problem").append(
"<div class='solution' data-type='custom'>" +
"<div class='instruction'>" +
"<input type='text' id='custom-input'>" +
"</div>" +
"<div class='guess'>$('#custom-input').val()</div>" +
"<div class='validator-function'>" +
"return guess == 'empty' ? '' : guess == 5;" +
"</div>" +
"</div>"
);

var answerData = Khan.answerTypes.custom.setup($("#solutionarea"),
$problem.children(".solution"));

testAnswer(answerData, "5", "right", "right answer is right");
testAnswer(answerData, "empty", "empty", "empty answer is empty");
testAnswer(answerData, "1", "wrong", "wrong answer is wrong");

start();
});

asyncTest("prime factorization", 24, function() {
setupSolutionArea();
Expand Down
4 changes: 2 additions & 2 deletions utils/answer-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,7 @@ Khan.answerTypes = $.extend(Khan.answerTypes, {
// otherwise correct or not before forwarding on the
// message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment here should probably be changed to reflect the new semantics of what is going on.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be still valid (the unsimplified fraction is the only case that the branch is taken that I'm aware of).

If a student enters "2/4" into a proper fraction field a "your answer is almost correct" message is normally displayed. (Under the default mode of simplification.) Such an answer does not fall under any of the three usual categories of "empty", "correct", or "incorrect", thus the framework uses a special score for it -- the score is marked as "empty", but carries the message.

Now the multiple exercise has a surprising semantic: if any subscore is "empty", but not all are, then the combined score is incorrect (even if all nonempty parts are actually correct; this happens as empty scores have correct: false). Without the branch with the lengthy comment, any multiple with an unsimplified part and another correct part would be graded wrong.

If we were to let multiple be empty whenever at least one of its parts is empty and all nonempty parts are correct, then this special case would be unnecessary -- a single unsimplified fraction would cause the combined score to be empty. And an empty score is treated as unanswered, displaying the message it carries. That's the content of wrwrwr@9a25e47 (not part of this PR).

blockGradingMessage = pass.message;
score.empty = false;
} else {
score.empty = score.empty && pass.empty;
score.correct = score.correct && pass.correct;
Expand All @@ -1232,7 +1233,6 @@ Khan.answerTypes = $.extend(Khan.answerTypes, {
guess: guess
};
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking my understanding... Is this the big killer that was keeping everything from being marked empty?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was keeping some things from being empty / unanswered.

First, at line 1196 there is the checkIfAnswerEmpty test. In simple cases it will work, but as it checks raw posted values (before letting subproblems process them) it's not reliable.

Second, if the multiple happens to have a rational-accepting subproblem (such as the default number type), that has required simplification (that's also the default), then entering an unsimplified correct result makes the score empty-with-message (through the block-grading-message-branch).

Third, there is the case of an empty score with an empty-string message, due to an unanswered custom subproblem (that is graded incorrect -- see below).

score.empty = false;
return score;
}
};
Expand Down Expand Up @@ -1877,7 +1877,7 @@ Khan.answerTypes = $.extend(Khan.answerTypes, {
return {
empty: pass === "",
correct: pass === true,
message: typeof pass === "string" ? pass : null,
message: typeof pass === "string" && pass !== "" ? pass : null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this wasn't here, would the "there are still more parts..." prompt not appear?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the simplest case of a custom-in-multiple (there actually are a few exercises with just this structure):

<div class="solution" data-type="multiple">
    <div class="sol" data-type="custom">
        <div class="instruction">Always unanswered</div>
        <div class="guess">[2]</div>
        <div class="validator-function">return '';</div>
    </div>
</div>

(here's a minimum working exercise: test-custom-in-multiple.html). As the validator function always returns an empty string the problem should be considered unanswered.

The current custom exercise code first checks that the result is not an object and then returns a score that looks like this: { empty=true, correct=false, guess=[1], message=""}. This becomes a "pass" in the multiple validator. The pass.message && pass.empty condition evaluates to an empty string and fails, thus the score (with its empty-string message, with or without empty set to false) becomes the exercise score.

Further, the score is processed in interface.js. The message is not null, so at line 477 attemptMessage is set to an empty string. However, the test at line 484 succeeds and the user is informed that the answer is incorrect.

guess: guess
};
}
Expand Down