Merge pull request #3489 from edx/jmclaus/bugfix_numerical_response_tolerance

Tolerance expressed in percentage now computes correctly. [BLD-522]

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Blades: Tolerance expressed in percentage now computes correctly. BLD-522.
+
 Studio: Add drag-and-drop support to the container page. STUD-1309.
 
 Common: Add extensible third-party auth module.

common/lib/capa/capa/tests/test_responsetypes.py

@@ -20,6 +20,7 @@ from capa.responsetypes import LoncapaProblemError, \
     StudentInputError, ResponseError
 from capa.correctmap import CorrectMap
 from capa.util import convert_files_to_filenames
+from capa.util import compare_with_tolerance
 from capa.xqueue_interface import dateformat
 
 from pytz import UTC
@@ -1120,7 +1121,6 @@ class NumericalResponseTest(ResponseTest):
     # We blend the line between integration (using evaluator) and exclusively
     # unit testing the NumericalResponse (mocking out the evaluator)
     # For simple things its not worth the effort.
-
     def test_grade_range_tolerance(self):
         problem_setup = [
             # [given_asnwer, [list of correct responses], [list of incorrect responses]]
@@ -1177,9 +1177,20 @@ class NumericalResponseTest(ResponseTest):
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
 
     def test_grade_percent_tolerance(self):
+        # Positive only range
         problem = self.build_problem(answer=4, tolerance="10%")
-        correct_responses = ["4.0", "4.3", "3.7", "4.30", "3.70"]
-        incorrect_responses = ["", "4.5", "3.5", "0"]
+        correct_responses = ["4.0", "4.00", "4.39", "3.61"]
+        incorrect_responses = ["", "4.41", "3.59", "0"]
+        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
+        # Negative only range
+        problem = self.build_problem(answer=-4, tolerance="10%")
+        correct_responses = ["-4.0", "-4.00", "-4.39", "-3.61"]
+        incorrect_responses = ["", "-4.41", "-3.59", "0"]
+        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
+        # Mixed negative/positive range
+        problem = self.build_problem(answer=1, tolerance="200%")
+        correct_responses = ["1", "1.00", "2.99", "0.99"]
+        incorrect_responses = ["", "3.01", "-1.01"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
 
     def test_floats(self):

common/lib/capa/capa/tests/test_util.py

+"""Tests capa util"""
+
+import unittest
+import textwrap
+from . import test_capa_system
+from capa.util import compare_with_tolerance
+
+
+class UtilTest(unittest.TestCase):
+    """Tests for util"""
+    def setUp(self):
+        super(UtilTest, self).setUp()
+        self.system = test_capa_system()
+
+    def test_compare_with_tolerance(self):
+        # Test default tolerance '0.001%' (it is relative)
+        result = compare_with_tolerance(100.0, 100.0)
+        self.assertTrue(result)
+        result = compare_with_tolerance(100.001, 100.0)
+        self.assertTrue(result)
+        result = compare_with_tolerance(101.0, 100.0)
+        self.assertFalse(result)
+        # Test absolute percentage tolerance
+        result = compare_with_tolerance(109.9, 100.0, '10%', False)
+        self.assertTrue(result)
+        result = compare_with_tolerance(110.1, 100.0, '10%', False)
+        self.assertFalse(result)
+        # Test relative percentage tolerance
+        result = compare_with_tolerance(111.0, 100.0, '10%', True)
+        self.assertTrue(result)
+        result = compare_with_tolerance(112.0, 100.0, '10%', True)
+        self.assertFalse(result)
+        # Test absolute tolerance (string)
+        result = compare_with_tolerance(109.9, 100.0, '10.0', False)
+        self.assertTrue(result)
+        result = compare_with_tolerance(110.1, 100.0, '10.0', False)
+        self.assertFalse(result)
+         # Test relative tolerance (string)
+        result = compare_with_tolerance(111.0, 100.0, '0.1', True)
+        self.assertTrue(result)
+        result = compare_with_tolerance(112.0, 100.0, '0.1', True)
+        self.assertFalse(result)
+        # Test absolute tolerance (float)
+        result = compare_with_tolerance(109.9, 100.0, 10.0, False)
+        self.assertTrue(result)
+        result = compare_with_tolerance(110.1, 100.0, 10.0, False)
+        self.assertFalse(result)
+         # Test relative tolerance (float)
+        result = compare_with_tolerance(111.0, 100.0, 0.1, True)
+        self.assertTrue(result)
+        result = compare_with_tolerance(112.0, 100.0, 0.1, True)
+        self.assertFalse(result)
+        ##### Infinite values #####
+        infinity = float('Inf')
+        # Test relative tolerance (float)
+        result = compare_with_tolerance(infinity, 100.0, 1.0, True)
+        self.assertFalse(result)
+        result = compare_with_tolerance(100.0, infinity, 1.0, True)
+        self.assertFalse(result)
+        result = compare_with_tolerance(infinity, infinity, 1.0, True)
+        self.assertTrue(result)
+        # Test absolute tolerance (float)
+        result = compare_with_tolerance(infinity, 100.0, 1.0, False)
+        self.assertFalse(result)
+        result = compare_with_tolerance(100.0, infinity, 1.0, False)
+        self.assertFalse(result)
+        result = compare_with_tolerance(infinity, infinity, 1.0, False)
+        self.assertTrue(result)
+        # Test relative tolerance (string)
+        result = compare_with_tolerance(infinity, 100.0, '1.0', True)
+        self.assertFalse(result)
+        result = compare_with_tolerance(100.0, infinity, '1.0', True)
+        self.assertFalse(result)
+        result = compare_with_tolerance(infinity, infinity, '1.0', True)
+        self.assertTrue(result)
+        # Test absolute tolerance (string)
+        result = compare_with_tolerance(infinity, 100.0, '1.0', False)
+        self.assertFalse(result)
+        result = compare_with_tolerance(100.0, infinity, '1.0', False)
+        self.assertFalse(result)
+        result = compare_with_tolerance(infinity, infinity, '1.0', False)
+        self.assertTrue(result)

common/lib/capa/capa/util.py

@@ -7,16 +7,29 @@ from cmath import isinf
 default_tolerance = '0.001%'
 
 
-def compare_with_tolerance(complex1, complex2, tolerance=default_tolerance, relative_tolerance=False):
+def compare_with_tolerance(student_complex, instructor_complex, tolerance=default_tolerance, relative_tolerance=False):
     """
-    Compare complex1 to complex2 with maximum tolerance tol.
+    Compare student_complex to instructor_complex with maximum tolerance tolerance.
 
-    If tolerance is type string, then it is counted as relative if it ends in %; otherwise, it is absolute.
+     - student_complex    :  student result (float complex number)
+     - instructor_complex    :  instructor result (float complex number)
+     - tolerance   :  float, or string (representing a float or a percentage)
+     - relative_tolerance: bool, to explicitly use passed tolerance as relative
 
-     - complex1    :  student result (float complex number)
-     - complex2    :  instructor result (float complex number)
-     - tolerance   :  string representing a number or float
-     - relative_tolerance: bool, used when`tolerance` is float to explicitly use passed tolerance as relative.
+     Note: when a tolerance is a percentage (i.e. '10%'), it will compute that
+     percentage of the instructor result and yield a number.
+
+     If relative_tolerance is set to False, it will use that value and the
+     instructor result to define the bounds of valid student result:
+     instructor_complex = 10, tolerance = '10%' will give [9.0, 11.0].
+
+     If relative_tolerance is set to True, it will use that value and both
+     instructor result and student result to define the bounds of valid student
+     result:
+     instructor_complex = 10, student_complex = 20, tolerance = '10%' will give
+     [8.0, 12.0].
+     This is typically used internally to compare float, with a
+     default_tolerance = '0.001%'.
 
      Default tolerance of 1e-3% is added to compare two floats for
      near-equality (to handle machine representation errors).
@@ -29,23 +42,28 @@ def compare_with_tolerance(complex1, complex2, tolerance=default_tolerance, rela
         In [212]: 1.9e24 - 1.9*10**24
         Out[212]: 268435456.0
     """
+    if isinstance(tolerance, str):
+        if tolerance == default_tolerance:
+            relative_tolerance = True
+        if tolerance.endswith('%'):
+            tolerance = evaluator(dict(), dict(), tolerance[:-1]) * 0.01
+            if not relative_tolerance:
+                tolerance = tolerance * abs(instructor_complex)
+        else:
+            tolerance = evaluator(dict(), dict(), tolerance)
+
     if relative_tolerance:
-        tolerance = tolerance * max(abs(complex1), abs(complex2))
-    elif tolerance.endswith('%'):
-        tolerance = evaluator(dict(), dict(), tolerance[:-1]) * 0.01
-        tolerance = tolerance * max(abs(complex1), abs(complex2))
-    else:
-        tolerance = evaluator(dict(), dict(), tolerance)
+        tolerance = tolerance * max(abs(student_complex), abs(instructor_complex))
 
-    if isinf(complex1) or isinf(complex2):
-        # If an input is infinite, we can end up with `abs(complex1-complex2)` and
+    if isinf(student_complex) or isinf(instructor_complex):
+        # If an input is infinite, we can end up with `abs(student_complex-instructor_complex)` and
         # `tolerance` both equal to infinity. Then, below we would have
         # `inf <= inf` which is a fail. Instead, compare directly.
-        return complex1 == complex2
+        return student_complex == instructor_complex
     else:
         # v1 and v2 are, in general, complex numbers:
         # there are some notes about backward compatibility issue: see responsetypes.get_staff_ans()).
-        return abs(complex1 - complex2) <= tolerance
+        return abs(student_complex - instructor_complex) <= tolerance
 
 
 def contextualize_text(text, context):  # private