Skip to content

Commit 01b8bae

Browse files
committed
use BigDecimal to compute numeric error margin
test plan: - create numeric answer questions with error margins - copy the course - in the copied quiz, the error margins should match the original course (show as 0.0001 and not something like 0.00009999999999889) fixes CNVS-19615 Change-Id: Iba9e40f9e37323310e6414794139c68d0f1c061c Reviewed-on: https://gerrit.instructure.com/51334 Reviewed-by: James Williams <jamesw@instructure.com> Tested-by: Jenkins QA-Review: Clare Strong <clare@instructure.com> Product-Review: Jeremy Stanley <jeremy@instructure.com>
1 parent 84215a4 commit 01b8bae

3 files changed

Lines changed: 47 additions & 8 deletions

File tree

gems/plugins/qti_exporter/lib/qti/numeric_interaction.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
require 'bigdecimal'
2+
13
module Qti
24
class NumericInteraction < AssessmentItemConverter
35
def initialize(opts={})
@@ -39,11 +41,13 @@ def get_canvas_answers
3941
exact_node = or_node.at_css('stringMatch baseValue')
4042
next unless exact_node
4143
answer[:numerical_answer_type] = 'exact_answer'
42-
exact = exact_node.text.to_f rescue 0.0
43-
answer[:exact] = exact
44+
exact = exact_node.text rescue "0.0"
45+
answer[:exact] = exact.to_f
4446
if upper = or_node.at_css('and customOperator[class=varlte] baseValue')
45-
margin = upper.text.to_f - exact rescue 0.0
46-
answer[:margin] = margin
47+
# do margin computation with BigDecimal to avoid rounding errors
48+
# (this is also used when _scoring_ numeric range questions)
49+
margin = BigDecimal.new(upper.text) - BigDecimal.new(exact) rescue "0.0"
50+
answer[:margin] = margin.to_f
4751
end
4852
@question[:answers] << answer
4953
elsif and_node = r_if.at_css('and')

lib/cc/qti/qti_items.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
# You should have received a copy of the GNU Affero General Public License along
1616
# with this program. If not, see <http://www.gnu.org/licenses/>.
1717
#
18+
require 'bigdecimal'
19+
1820
module CC
1921
module QTI
2022
module QTIItems
@@ -340,9 +342,10 @@ def numerical_resprocessing(node, question)
340342
or_node.varequal exact, :respident=>"response1"
341343
unless answer['margin'].blank?
342344
or_node.and do |and_node|
343-
margin = answer['margin'].to_f
344-
and_node.vargte(exact - margin, :respident=>"response1")
345-
and_node.varlte(exact + margin, :respident=>"response1")
345+
exact = BigDecimal.new(answer['exact'].to_s)
346+
margin = BigDecimal.new(answer['margin'].to_s)
347+
and_node.vargte((exact - margin).to_f, :respident=>"response1")
348+
and_node.varlte((exact + margin).to_f, :respident=>"response1")
346349
end
347350
end
348351
end

spec/models/content_migration/course_copy_quizzes_spec.rb

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,38 @@
650650
expect(decoy_assignment_group.reload.name).not_to eql group.name
651651
end
652652

653+
it "should round numeric answer margins sanely" do
654+
q = @copy_from.quizzes.create!(:title => "blah")
655+
# this one targets rounding errors in gems/plugins/qti_exporter/lib/qti/numeric_interaction.rb (import side)
656+
data1 = {:question_type => "numerical_question",
657+
:question_text => "what is the optimal matter/antimatter intermix ratio",
658+
:answers => [{
659+
:text => "answer_text",
660+
:weight => 100,
661+
:numerical_answer_type => "exact_answer",
662+
:answer_exact => 1,
663+
:answer_error_margin => 0.0001
664+
}]}.with_indifferent_access
665+
# this one targets rounding errors in lib/cc/qti/qti_items.rb (export side)
666+
data2 = {:question_type => "numerical_question",
667+
:question_text => "what is the airspeed velocity of an unladed African swallow",
668+
:answers => [{
669+
:text => "answer_text",
670+
:weight => 100,
671+
:numerical_answer_type => "exact_answer",
672+
:answer_exact => 2.0009,
673+
:answer_error_margin => 0.0001
674+
}]}.with_indifferent_access
675+
676+
q.quiz_questions.create!(:question_data => data1)
677+
q.quiz_questions.create!(:question_data => data2)
678+
run_course_copy
679+
680+
q2 = @copy_to.quizzes.where(migration_id: mig_id(q)).first
681+
expect(q2.quiz_questions[0].question_data["answers"][0]["margin"].to_s).to eq "0.0001"
682+
expect(q2.quiz_questions[1].question_data["answers"][0]["margin"].to_s).to eq "0.0001"
683+
end
684+
653685
it "should not combine when copying question banks with the same title" do
654686
data = {'question_name' => 'test question 1', 'question_type' => 'essay_question', 'question_text' => 'blah'}
655687

@@ -685,7 +717,7 @@
685717
expect(group2_copy.assessment_question_bank_id).to eq bank2_copy.id
686718
end
687719

688-
it "hould copy stuff" do
720+
it "should copy stuff" do
689721
data1 = {:question_type => "file_upload_question",
690722
:points_possible => 10,
691723
:question_text => "why is this question terrible"

0 commit comments

Comments
 (0)