Share one numpy.isclose tolerance check between the symbolic and physical quantity contexts

app/context/physical_quantity/graphs/match.py

@@ -17,6 +17,7 @@
     is_proportional,
     parse_criterion_sides,
 )
+from app.context.tolerance import values_within_tolerance
 from app.evaluation_parameters import EvaluationParameters
 from app.feedback.physical_quantity import feedback_string_generators as physical_quantity_feedback_string_generators
 
@@ -73,15 +74,12 @@ def _within_tolerance(parameters: EvaluationParameters, lhs_string: str, rhs_str
     ans = ans_value.simplify()
     res = res_value.simplify()
 
-    value_match = False
+    if not (ans.is_constant() and res.is_constant()):
+        return False
 
-    if ans.is_constant() and res.is_constant():
-        if rtol > 0 and (ans != 0):
-            value_match = bool(abs(float((ans - res) / ans)) < rtol)
-        elif atol > 0 or (ans == 0):
-            answer_unit_factor = float(parameters.reserved_expressions["answer"].quantity.unit_factor)
-            value_match = bool(abs(float(ans - res)) < atol * answer_unit_factor)
-    return value_match
+    # The standard values are in SI base units, while atol is given in the units of the answer
+    answer_unit_factor = float(parameters.reserved_expressions["answer"].quantity.unit_factor)
+    return values_within_tolerance(res, ans, rtol, atol * answer_unit_factor)
 
 
 def _quantity_match(parameters: EvaluationParameters, label: str, sides: CriterionSides, rtol: float, atol: float) -> Evaluate:

app/context/symbolic/numerical.py

@@ -2,14 +2,15 @@
 
 from sympy import Basic, pi, N
 
+from app.context.tolerance import values_within_tolerance
 from app.evaluation_parameters import EvaluationParameters
 
 def compare_numerically(lhs_expr: Basic, rhs_expr: Basic, parameters: EvaluationParameters) -> bool:
     """
     Compares `lhs_expr` and `rhs_expr` numerically using the `atol` and `rtol` parameters.
 
     Returns False if numerical comparison is not enabled (`numerical` is not set and neither
-    `atol` nor `rtol` is positive), otherwise whether the difference is within both tolerances.
+    `atol` nor `rtol` is positive), otherwise whether the difference is within the tolerances.
     """
     if not (parameters.numerical
             or float(parameters.rtol) > 0
@@ -29,20 +30,4 @@ def replace_pi(expr: Basic) -> Basic:
     # Separates LHS and RHS, parses and evaluates them
     res = N(replace_pi(lhs_expr))
     ans = N(replace_pi(rhs_expr))
-    if float(parameters.atol) > 0:
-        try:
-            absolute_error = abs(float(ans - res))
-            error_below_atol = bool(absolute_error < float(parameters.atol))
-        except TypeError:
-            error_below_atol = None
-    else:
-        error_below_atol = True
-    if float(parameters.rtol) > 0:
-        try:
-            relative_error = abs(float((ans - res) / ans))
-            error_below_rtol = bool(relative_error < float(parameters.rtol))
-        except TypeError:
-            error_below_rtol = None
-    else:
-        error_below_rtol = True
-    return error_below_atol is True and error_below_rtol is True
+    return values_within_tolerance(res, ans, float(parameters.rtol), float(parameters.atol))

app/context/tolerance.py

@@ -0,0 +1,24 @@
+"""The numerical tolerance check shared by the symbolic and physical quantity contexts."""
+
+from __future__ import annotations
+
+from sympy import Basic
+
+
+def values_within_tolerance(res: Basic, ans: Basic, rtol: float, atol: float) -> bool:
+    """
+    Whether `res` is within the tolerances of `ans`, using the same rule as numpy.isclose:
+    |ans - res| <= atol + rtol*|ans|.
+
+    If `ans` and `res` still contain symbols that cancel in (ans - res)/ans (e.g. both are multiples of rho*L**3),
+    only the relative tolerance is checked, since atol needs a numerical value of `ans`.
+    Returns False when the values cannot be compared numerically.
+    """
+    try:
+        return bool(abs(float(ans - res)) <= atol + rtol * abs(float(ans)))
+    except TypeError:
+        pass
+    try:
+        return bool(abs(float((ans - res) / ans)) <= rtol)
+    except (TypeError, ZeroDivisionError):
+        return False

app/docs/user.md

@@ -11,7 +11,7 @@ Note that this function is designed to handle comparisons of mathematical expres
 There are 15 optional parameters that can be set: `absolute_tolerance`, `complexNumbers`, `convention`, `criteria`, `elementary_functions`, `feedback_for_incorrect_response`, `multiple_answers_criteria`, `physical_quantity`, `plus_minus`/`minus_plus`, `rtol`, `specialFunctions`, `strict_syntax`, `strictness`, `symbol_assumptions`.
 
 #### `absolute_tolerance` (`atol`)
-Sets the absolute tolerance, $e_a$, i.e. if the answer, $x$, and response, $\tilde{x}$, are numerical values then the response is considered equal to the answer if $|x-\tilde{x}| \leq e_aBy default `absolute_tolerance` is set to `0`, which means the comparison will be done with as high accuracy as possible. If either the answer or the response aren't numerical expressions this parameter is ignored.
+Sets the absolute tolerance, $e_a$, i.e. if the answer, $x$, and response, $\tilde{x}$, are numerical values then the response is considered equal to the answer if $|x-\tilde{x}| \leq e_a$. If `relative_tolerance`, $e_r$, is also set, the two combine into $|x-\tilde{x}| \leq e_a + e_r|x|$ (the same rule as `numpy.isclose`). For physical quantities, $e_a$ is in the units of the answer. By default `absolute_tolerance` is set to `0`, which means the comparison will be done with as high accuracy as possible. If either the answer or the response aren't numerical expressions this parameter is ignored.
 
 #### `complexNumbers`
 
@@ -77,7 +77,7 @@ When `physical_quantity` the evaluation function will generate feedback based on
 **TODO:** Generate new flowchart for updated physical quantity feedback generation procedure.
 
 #### `relative_tolerance` (`rtol`)
-Sets the relative tolerance, $e_r$, i.e. if the answer, $x$, and response, $\tilde{x}$, are numerical values then the response is considered equal to the answer if $\left|\frac{x-\tilde{x}}{x}\right| \leq e_r$. By default `relative_tolerance` is set to `0`, which means the comparison will be done with as high accuracy as possible. If either the answer or the response aren't numerical expressions this parameter is ignored.
+Sets the relative tolerance, $e_r$, i.e. if the answer, $x$, and response, $\tilde{x}$, are numerical values then the response is considered equal to the answer if $\left|\frac{x-\tilde{x}}{x}\right| \leq e_r$. If `absolute_tolerance` is also set, see above for how the two combine. By default `relative_tolerance` is set to `0`, which means the comparison will be done with as high accuracy as possible. If either the answer or the response aren't numerical expressions this parameter is ignored.
 
 #### `strictness`
 
@@ -338,9 +338,11 @@ In the examples set there is one response area for each of the two conventions,
 
 The evaluation function can be configured to do numerical comparisons in the following ways:
 
-- Setting the absolute error tolerance: This is done by setting the parameter `atol`. When `atol` is set to any positive value, then `response` will be considered correct if $|$`answer`$-$`response`$| < $`atol`.
+- Setting the absolute error tolerance: This is done by setting the parameter `atol`. When `atol` is set to any positive value, then `response` will be considered correct if $|$`answer`$-$`response`$| \leq $`atol`.
 
-- Setting the relative error tolerance: This is done by setting the parameter `rtol`. When `rtol` is set to any positive value, then `response` will be considered correct if $|$`answer`$-$`response`$|/|$`answer`$| < $`rtol`.
+- Setting the relative error tolerance: This is done by setting the parameter `rtol`. When `rtol` is set to any positive value, then `response` will be considered correct if $|$`answer`$-$`response`$|/|$`answer`$| \leq $`rtol`.
+
+- Setting both: `response` will be considered correct if $|$`answer`$-$`response`$| \leq $`atol`$ + $`rtol`$\cdot|$`answer`$|$, the same rule as `numpy.isclose`.
 
 **Note:** If the response area is used to compare physical quantities (i.e. the parameter `physical_quantity` is set to `true`) comparisons will be numerical be default (since unit conversion has finite precision).
 

app/tests/physical_quantity_evaluation_test.py

@@ -595,6 +595,24 @@ def test_dimension_matches_compares_only_dimensions(self, res, criteria, is_corr
         result = evaluation_function(res, "1.0 m", params)
         assert result["is_correct"] is is_correct
 
+    @pytest.mark.parametrize(
+        "res,ans,tolerances,is_correct",
+        [
+            # Only rtol used to be checked when both were set (bug 9 in the bug report),
+            # now they combine as in numpy.isclose: |answer - response| <= atol + rtol*|answer|
+            ("1.55 m", "1 m", {"rtol": 0.1, "atol": 0.5}, True),
+            ("1.65 m", "1 m", {"rtol": 0.1, "atol": 0.5}, False),
+            # A difference equal to the tolerance used to be rejected
+            ("1.5 m", "1 m", {"atol": 0.5}, True),
+            # atol is in the units of the answer
+            ("1.004 km", "1 km", {"atol": 0.005}, True),
+            ("1.006 km", "1 km", {"atol": 0.005}, False),
+        ]
+    )
+    def test_numerical_tolerances(self, res, ans, tolerances, is_correct):
+        params = {'strict_syntax': False, 'physical_quantity': True, 'elementary_functions': True, **tolerances}
+        assert evaluation_function(res, ans, params)["is_correct"] is is_correct
+
     @pytest.mark.parametrize(
         "res,ans,criteria,is_correct",
         [

app/tests/symbolic_evaluation_test.py

@@ -1,7 +1,8 @@
 import pytest
 
-from sympy import I, Integer, Symbol, cos, oo, parse_expr, sin, zoo
+from sympy import Float, I, Integer, Symbol, cos, oo, parse_expr, sin, zoo
 
+from app.context.tolerance import values_within_tolerance
 from app.context.symbolic.comparison import do_comparison_infinite, normalise_complex
 from app.context.symbolic.graphs.substitution import _group_values
 from app.context.symbolic.variations import (
@@ -2251,6 +2252,40 @@ def test_tolerance_aliases_do_not_change_params(self, alias, tolerance):
         del params[alias]
         assert evaluation_function("1.05", "1", params)["is_correct"] is False
 
+    @pytest.mark.parametrize(
+        "response,answer,tolerances,is_correct",
+        [
+            # Both tolerances used to have to pass separately (bug 9 in the bug report),
+            # now they combine as in numpy.isclose: |answer - response| <= atol + rtol*|answer|
+            ("1.55", "1", {"rtol": 0.1, "atol": 0.5}, True),
+            ("1.65", "1", {"rtol": 0.1, "atol": 0.5}, False),
+            # A difference equal to the tolerance used to be rejected
+            ("1.5", "1", {"atol": 0.5}, True),
+            # Symbols that cancel in (answer - response)/answer are compared with rtol, also when atol is set
+            ("1.05*rho*L**3", "rho*L**3", {"rtol": 0.1}, True),
+            ("1.05*rho*L**3", "rho*L**3", {"rtol": 0.1, "atol": 0.01}, True),
+            ("1.2*rho*L**3", "rho*L**3", {"rtol": 0.1, "atol": 0.01}, False),
+        ]
+    )
+    def test_numerical_tolerances(self, response, answer, tolerances, is_correct):
+        params = {'strict_syntax': False, 'elementary_functions': True, **tolerances}
+        assert evaluation_function(response, answer, params)["is_correct"] is is_correct
+
+    @pytest.mark.parametrize(
+        "res,ans,rtol,atol,expected",
+        [
+            (Float(1.55), Integer(1), 0.1, 0.5, True),
+            (Float(1.65), Integer(1), 0.1, 0.5, False),
+            (Integer(0), Integer(0), 0.1, 0, True),
+            (Float(0.1), Integer(0), 0.1, 0, False),
+            # Values that cannot be converted to float are not within tolerance
+            (I, Integer(1), 0.1, 0.5, False),
+            (Symbol('x'), Symbol('y'), 0.1, 0.5, False),
+        ]
+    )
+    def test_values_within_tolerance(self, res, ans, rtol, atol, expected):
+        assert values_within_tolerance(res, ans, rtol, atol) is expected
+
     def test_feedback_follows_criteria_order(self):
         # The order used to depend on memory addresses (bug 13 in the bug report), so check several runs
         params = {