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

Commit

Permalink
Be more conservative about saying something is a multiple of pi.
Browse files Browse the repository at this point in the history
Summary:
I can't figure out how 'pi' is being set as a possible answer-form for
questions like
   https://www.khanacademy.org/math/cc-fourth-grade-math/cc-4th-place-value-rounding/cc-4th-rounding/e/rounding-whole-numbers-2

But it is, and rather than have to fix that up, and then trust editors
to properly say "this answer involves pi" at all, let's just be better
about guessing whether a number is a multiple of pi.

This is probably not perfect, but I think is good enough.  If we're
worried this will suppress legitimate messaging, we could just
suppress it if the answer is an integer, which I expect will catch 95%
of the confusing cases now.

This resolves https://app.asana.com/0/27216215224639/96623486099021/f

This also resolves T2578

Test Plan:
Ran `python -m SimpleHTTPServer` and visited localhost:8000/test

Also will try the rounding-whole-numbers questions after this is deployed.
I expect not to get a "approximated pi" warning if I say xxxx0001 as
an answer.

Reviewers: eater

Reviewed By: eater

Subscribers: jared

Maniphest Tasks: T2578

Differential Revision: https://phabricator.khanacademy.org/D26043
  • Loading branch information
csilvers committed Mar 21, 2016
1 parent 9b32d7c commit 5b40b89
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
10 changes: 9 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.FAKE: pack packed lint fix_lint
.FAKE: serve pack packed lint fix_lint

# This needs 'npm' on your system, to install jison.
# The output file has to be named 'Calculator' because that's how jison
Expand All @@ -11,6 +11,14 @@ genfiles/calculator.js: build/calculator/calculator.jison build/calculator/calcu
mv genfiles/Calculator.js genfiles/calculator.js


serve:
python -m SimpleHTTPServer


check:
echo "Run 'make serve' then visit http://localhost:8000/test"


deps:
npm install

Expand Down
30 changes: 30 additions & 0 deletions test/answer-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -1363,4 +1363,34 @@
}, 400);
});

asyncTest("number pi (approximation is not overzealous 1)", 6, function() {
setupSolutionArea();
var $problem = jQuery("#qunit-fixture .problem").append(
"<p class='solution' data-type='number' data-forms='pi'>565.5</p>"
);

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

testAnswer(answerData, "565.5", "right", "right answer is right");
testAnswer(answerData, "565", "wrong", "wrong answer is wrong");

start();
});

asyncTest("number pi (approximation is not overzealous 2)", 6, function() {
setupSolutionArea();
var $problem = jQuery("#qunit-fixture .problem").append(
"<p class='solution' data-type='number' data-forms='pi'>0.833333333333333333</p>"
);

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

testAnswer(answerData, "5/6", "right", "right answer is right");
testAnswer(answerData, ".833", "wrong", "wrong answer is wrong");

start();
});

})();
28 changes: 25 additions & 3 deletions utils/answer-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,31 @@ Khan.answerTypes = $.extend(Khan.answerTypes, {
possibilities = _.reduce(Khan.answerTypes.predicate.defaultForms.split(/\s*,\s*/), function(memo, form) {
return memo.concat(forms[form](text));
}, []);
$.each(possibilities, function(ix, possibility) {
possibility.piApprox = true;
});
// If the answer is a floating point number that's
// near a multiple of pi, mark is as being possibly
// an approximation of pi. We actually check if
// it's a plausible approximation of pi/12, since
// sometimes the correct answer is like pi/3 or pi/4.
// We also say it's a pi-approximation if it involves
// x/7 (since 22/7 is an approximation of pi.)
// Never mark an integer as being an approximation
// of pi.
var approximatesPi = false;
var number = parseFloat(text);
if (!isNaN(number) && number !== parseInt(text)) {
var piMult = Math.PI / 12;
var roundedNumber = piMult * Math.round(number / piMult);
if (Math.abs(number - roundedNumber) < 0.01) {
approximatesPi = true;
}
} else if (text.match(/\/\s*7/)) {
approximatesPi = true;
}
if (approximatesPi) {
$.each(possibilities, function(ix, possibility) {
possibility.piApprox = true;
});
}
return possibilities;
}

Expand Down

0 comments on commit 5b40b89

Please sign in to comment.